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