RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

Claes Redestad claes.redestad at oracle.com
Sun Apr 10 04:36:03 UTC 2016

Hi Steve,

checking that there's a newline after the true seems OK according to the 
grammar[1], so that seems like a good call.

Not sure it really matters, but perhaps setting isMultiRelease to true 
once the test passes improves readability:

                 if (MULTI_RELEASE_ENABLED && version != BASE_VERSION) {
                     int i = match(MULTIRELEASE_CHARS, b, 
                     if (i != -1) {
                         i += MULTIRELEASE_CHARS.length;
                         if (i < b.length) {
                             byte c = b[i];
                             if (c == '\r' || c == '\n') {
                                 isMultiRelease = true;

For completeness, maybe we should also check that the value is not part 
of a continuation:

                             // Check that  the value is followed by a
                             // newline and does not have a continuation
                             if ((c == '\r' || c == '\n') &&
                                     (i + 1 == b.length || b[i + 1] != ' 
')) {
                                 isMultiRelease = true;




On 2016-04-08 21:23, Steve Drach wrote:
> The new webrev is 
> http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
> <http://cr.openjdk.java.net/%7Esdrach/8153213/webrev/index.html>
> and the issue is https://bugs.openjdk.java.net/browse/JDK-8153213
> Now the value “true” followed by either ‘\r’ or ‘\n’ is the only 
> acceptable value.
>> On Apr 8, 2016, at 11:34 AM, Steve Drach <steve.drach at oracle.com 
>> <mailto:steve.drach at oracle.com>> wrote:
>> Okay, I’ll prepare a new webrev.  I think all we need to check for 
>> after “true” is \r or \n.  If the manifest just ends without at least 
>> one of those, it’s not a legal manifest.
>>> On Apr 8, 2016, at 11:25 AM, Claes Redestad 
>>> <claes.redestad at oracle.com <mailto:claes.redestad at oracle.com>> wrote:
>>> On 04/08/2016 07:54 PM, Steve Drach wrote:
>>>>>>>> I’ve put up a new webrev that addresses the issue of having 
>>>>>>>> spaces before (and after) the value “true” in the Multi-Release 
>>>>>>>> attribute.
>>>>>>> Is some or all of that really necessary? since the we can 
>>>>>>> specify domain of values.
>>>>>> I think it is.  The spec states that one can have an arbitrary 
>>>>>> amount of leading/trailing spaces in the value part of the 
>>>>>> attribute.  Apparently attribute values could also have “hidden” 
>>>>>> characters such as vertical tab or nak for example.  I wish the 
>>>>>> spec was tighter here.  I’m merely stating the the domain can be
>>>>>> “ *TRUE *” for upper/lower case letters.
>>>>> AFAICT the so called “spec" says nothing about trimming white 
>>>>> space from values and although not explicitly called out the 
>>>>> actual value has to be what constitutes the character sequence for 
>>>>> the “otherchar" definition (otherwise the continuation space and 
>>>>> newline characters would also be part of the actual value and that 
>>>>> makes no sense).
>>>> No it doesn’t say you can trim white space, but if one doesn’t, 
>>>> then do we accept “true”, “ true”, “  true”, etc.?
>>>>> So it seems you may have quite a bit of wiggle room here :-) and 
>>>>> it’s up to each attribute definition to state what constitutes its 
>>>>> domain of possible valid values based on “other char”.
>>>> So, we can say *otherchar is upper/lower case “true” only?
>>>>>>> For example, the Sealed attribute can take on a value of “true” 
>>>>>>> or “false” (case ignored), but there is no white space trimming.
>>>>>> Well then “Sealed:   true” won’t work, but the spec says it 
>>>>>> should work.
>>>>> Where does the “spec” state that it should?
>>>> It really comes down to the interpretation of otherchar.  If it can 
>>>> be interpreted on an attribute by attribute basis then it’s not a 
>>>> problem.
>>>>> The value is literally the string “ true”, at least in one 
>>>>> interpretation of the “spec” :-)
>>>> I’m fine with that.  And we really should do something about the 
>>>> spec, it’s too loose and ambiguous.
>>>>> I suspect it’s ok to go with finding “Multi-Release: true” 
>>>>> (lower/upper case) as in your first patch, then check if it is 
>>>>> terminated with a newline (and ignore the continuation case)
>>>> Okay.  I’d like Claes to weigh in because he’s the one who brought 
>>>> it up.  He’s traveling today, so I don’t expect to hear from him soon.
>>> Yeah, I'm guilty of raising the silly question of whether or not 
>>> accepting untrimmed true is OK according to spec.
>>> With a generous interpretation and the presence of prior art 
>>> (Sealed) I think it's OK to go back to the first version of the 
>>> patch (with the addition to check that Multi-Release: true is 
>>> followed by a LF, CR or the end of the manifest) and add a similar 
>>> note to the spec/notes that Multi-Release has the same value 
>>> restrictions as Sealed.
>>> Perhaps both cases should be clarified in the notes to say that 
>>> leading/trailing whitespace is not accepted, since that isn't 
>>> directly obvious to a casual reader.
>>> Thanks!
>>> /Claes
>>>>> Paul.
>>>>>> I am fine without doing this nonsense, but I think we need to 
>>>>>> change the spec to make it correct.  We could change the 
>>>>>> definition of "otherchar” to something like this
>>>>>> otherchar:=ALPHA EXTALPHANUM*
>>>>>> ALPHA:=[A-Z | a-z | _]
>>>>>> Even this will still allow trailing spaces, so after matching 
>>>>>> TRUE we’d need to make sure the next char is SPACE, \r, or \n.
>>>>>> Other ideas?
>>>>>>> Paul.
>>>>>>>>> Hi,
>>>>>>>>> Please review this simple fix to require that the jar manifest 
>>>>>>>>> Multi-Release attribute has a value of “true" in order to be 
>>>>>>>>> effective, i.e. to assert the jar is a multi-release jar.
>>>>>>>>> issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8153213>
>>>>>>>>> webrev: 
>>>>>>>>> http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>>>>>>>> <http://cr.openjdk.java.net/%7Esdrach/8153213/webrev/index.html> 
>>>>>>>>> <http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>>>>>>>> <http://cr.openjdk.java.net/%7Esdrach/8153213/webrev/index.html>>
>>>>>>>>> Thanks
>>>>>>>>> Steve

More information about the core-libs-dev mailing list