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
