Review/comment needed for the new public java.util.Base64 class

[Xueming Shen]

On 10/10/12 8:39 PM, Weijun Wang wrote:
>
>
> On 10/11/2012 11:32 AM, Xueming Shen wrote:
>> On 10/10/12 8:16 PM, Weijun Wang wrote:
>>>
>>>
>>> On 10/11/2012 11:09 AM, Xueming Shen wrote:
>>>> On 10/10/12 6:51 PM, Weijun Wang wrote:
>>>>> Several questions:
>>>>>
>>>>> 1. In encode0(byte[] src, byte[] dst)
>>>>>
>>>>>  281                 if (linepos == linemax && (atom != 0 || sp <
>>>>> sl)) {
>>>>>
>>>>> Maybe atom != 0 is not necessary?
>>>>
>>>> The logic here is that if we reached the last atom (atom == 0), but if
>>>> there
>>>> is still byte(s) in src (sp < sl), we will need to output the last
>>>> special unit,
>>>> which has one or two padding charactere '=', in this case, we still
>>>> need to
>>>> output the line separator(s).
>>>
>>> But when atom != 0, it seems sp < sl should always be true.
>>
>> Yes, the sp < sl part only counts if atom == 0. Means if it's NOT last
>> atom,
>> (atom != 0) output the line feeds, if it IS the last atom (atom == 
>> 0), only
>> output the line feeds if there are leftover byte (sp < sl), which 
>> means the
>> last 4-byte unit (with one or two '=') will be after this line feed.
>
> So, the value of sp < sl is always the same as (atom != 0 || sp < sl). 
> That's why I said atom != 0 is not necessary.
>
You're absolutely correct! Somehow I kept thinking you were saying sp < 
sl is not necessary:-)
not the other way around.

webrev will be updated shortly.

Thanks!
-Sherman



> -Max
>
>>
>> -Sherman
>>
>>>
>>> -Max
>>>
>>>>
>>>>>
>>>>> 2. Is it necessary to explicitly mention in the spec that there is no
>>>>> CrLf at the end of a MIME encoded string?
>>>>
>>>> I'm struggling with which is the appropriate/desired behavior, output
>>>> the crlf for the last line
>>>> or not. Apache's common coder appears to append the crlf for the last
>>>> line, but our sun.misc
>>>> version does not (but sun.misc.BASE64 actually appends the line
>>>> separator if the last line
>>>> happens to have 76 characters, I would assume this is a bug 
>>>> though). The
>>>> current implement
>>>> tries to match the sun.misc. I'm happy to go either way.
>>>>
>>>> But, as you suggested, it might be worth explicitly describing 
>>>> whatever
>>>> behavior we choose.
>>>>
>>>>>
>>>>> 3. The test confirms decoding can correctly reverse the encoding but
>>>>> it says nothing about the correctness of the encoding. Maybe we can
>>>>> just use "10.  Test Vectors" of RFC 4648?
>>>>>
>>>>
>>>> I do have a version of TestBase64 actually compares encoded results of
>>>> j.u.Base64,
>>>> sun.misc.BASE64Encoder and org.apache.commons.codec.binary.Base64. 
>>>> Maybe
>>>> I should
>>>> at least plug in the code for comparing with sun.misc.Base64Encoder.
>>>>
>>>> Thanks!
>>>> -Sherman
>>>>
>>>>
>>>>>
>>>>>
>>>>> On 10/11/2012 01:54 AM, Xueming Shen wrote:
>>>>>>
>>>>>> A standard/public API for Base64 encoding and decoding has been long
>>>>>> overdue. JDK8  has a JEP [1] for this particular request.
>>>>>>
>>>>>> Here is the draft proposal to add a public Base64 utility class for
>>>>>> JDK8.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~sherman/4235519/webrev
>>>>>>
>>>>>> This class basically provides 4 variants of Base64 encoding scheme,
>>>>>>
>>>>>> (1) the default RFC 4648, which uses standard mapping, no line 
>>>>>> breaks,
>>>>>> (2) the URL-safe RFE 4648, no line breaks, use "-" and "_" to 
>>>>>> replace
>>>>>> "+" and
>>>>>>      "/" for the mapping
>>>>>> (3) the default MIME style, as in RFC 2045 (and its earlier 
>>>>>> versions),
>>>>>> which uses
>>>>>>      "standard" base64 mapping, a 76 characters per line limit and
>>>>>> uses
>>>>>> crlf pair
>>>>>>       \r\n for line break.
>>>>>> (4) an extend-able MIME base64, with the char-per-line and the line
>>>>>> separator(s)
>>>>>>       specified by developer.
>>>>>>
>>>>>> The encoder/decoder now provides encoding and decoding for byte[],
>>>>>> String,
>>>>>> ByteBuffer and a pair of "EncoderInputStream" and
>>>>>> "DecoderOutputStrream",
>>>>>> which we believe/hope should satisfy most of the common use cases.
>>>>>> Additional
>>>>>> method(s) can be added if strongly desired.
>>>>>>
>>>>>> We tried couple slightly different styles of design for such this
>>>>>> "simple" utility
>>>>>> class [2].  We kinda concluded that the version proposed probably
>>>>>> provides
>>>>>> the best balance among readability, usability and extensibility.
>>>>>>
>>>>>> Please help review and comment on the API and implementation.
>>>>>>
>>>>>> Thanks!
>>>>>> -Sherman
>>>>>>
>>>>>> [1] http://openjdk.java.net/jeps/135
>>>>>> [2] http://cr.openjdk.java.net/~sherman/base64/
>>>>
>>