RFR: JDK-8007799 Base64.getEncoder(0, byte[]) returns encoder that unexpectedly inserts line separator when using some of the encoding methods

[Mark Sheppard]

Hi Sherman,
    I didn't think "outside the box" on this one, but was  a bit 
mechanical with the fix.

Your latter two options are appealing: returning Encoder.RFC4648 or
throwing an exception.

In fact your second option of using the Base64.getEncoder(),
ignoring the line separator and returning the RFC4648 encoder, solves 
the issue
neatly, and you can leave the spec as is :-)

As such:

    public static Encoder getEncoder(int lineLength, byte[] lineSeparator) {
         if (lineLength <= 0) {
             return Encoder.RFC4648;
         }
         Objects.requireNonNull(lineSeparator);
         int[] base64 = Decoder.fromBase64;
         for (byte b : lineSeparator) {
             if (base64[b & 0xff] != -1)
                 throw new IllegalArgumentException(
                         "Illegal base64 line separator character 0x"
                                 + Integer.toString(b, 16));
         }
         return new Encoder(false, lineSeparator, lineLength >> 2 << 2);
     }

will do the job nicely.

While throwing an exception will ensure a single point of access to the 
RFC4648 Encoder, and mean the
getEncoder(int, byte[]) is providing a genuine alternative Encoder to 
the three "build in" Encoders.
Thus, eliminating the lineLength <=0 option makes sense.

The former has least impact as a fix. The latter makes more sense when 
you think about it.

Whichever you favour and I'll make the change.

regards
Mark

On 02/04/2013 18:52, Xueming Shen wrote:
> Thanks Mark for looking into this issue.
>
> The change itself looks fine. Though I'm a little concerned whether
> the performance cost (need to do an additional linemax > 0 check
> inside the 'while" loop) is really worth it. Maybe an alternative is
> to simply set the linemax to -1 if it's 0? such as
>
>     public static Encoder getEncoder(int lineLength, byte[] 
> lineSeparator) {
>          Objects.requireNonNull(lineSeparator);
>          int[] base64 = Decoder.fromBase64;
>          for (byte b : lineSeparator) {
>              if (base64[b & 0xff] != -1)
>                  throw new IllegalArgumentException(
>                      "Illegal base64 line separator character 0x" + 
> Integer.toString(b, 16));
>          }
>          if (lineLength == 0)
>              lineLength = -1;
>          return new Encoder(false, lineSeparator, lineLength >> 2 << 2);
>     }
>
> And given it actually become a "normal" RFC4648 if the lineLength <=0, 
> maybe
> we should simply eliminate this "option" by changing the spec to say 
> "throw
> IAE if lineLength <=0", go use BAse64.getEncoder() if lineSeparator is 
> not needed.
>
> -Sherman
>
>
> On 03/27/2013 09:01 AM, Mark Sheppard wrote:
>> Hi,
>>    please oblige and review the webrev below as a fix for the issue 
>> raised in JDK-8007799
>>
>> http://cr.openjdk.java.net/~msheppar/8007799/webrev.00/
>>
>> Description:
>> "Specification for the method
>> java.util.Base64.getEncoder(int lineLength, byte[] lineSeparator)
>>
>> says:
>> Parameters:
>>     lineLength - the length of each output line (rounded down to 
>> nearest multiple of 4). If lineLength <= 0 the output will not be 
>> separated in lines
>>
>> However if a zero line length is specified encoding methods wrap() 
>> and encode(ByteBuffer src, ByteBuffer dst, int bytesOut) return 
>> encoded string which starts from the given line separator. "
>>
>> the patch adds a check for linemax > 0 whenever a line separator 
>> might be added, and adds an new test case.
>>
>> regards
>> Mark
>