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

[Paul Sandoz]

> On 12 Apr 2016, at 10:18, Claes Redestad  wrote:
> 
> Hi Steve,
> 
> On 2016-04-11 23:21, Steve Drach wrote:
>> Hi,
>> 
>> I’ve updated the following patch, incorporating code by Claes Redestad to 
>> handle some corner cases while processing the attribute value.  Note that 
>> we’ve limited the location of the value part of the attribute to accommodate 
>> startup performance requirements.  It’s not without precedent as at least 
>> one other attribute is also limited in amount of whitespace surrounding the 
>> value.
>> 
>>> 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 
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev.01/index.html 
>> 
>> 
> 
> this looks good to me.

Same here, and i like the additional tests. I think we have beaten this into 
submission at this point :-)

Paul.

> Good catch to delete the new jars added by the test.
> 
> I'll sponsor this for you once tests look OK.
> 
> Thanks!
> 
> /Claes
> 
>> 
>> Thanks
>> Steve
> 

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

[Claes Redestad]

Hi Steve,

On 2016-04-11 23:21, Steve Drach wrote:

Hi,

I’ve updated the following patch, incorporating code by Claes Redestad 
to handle some corner cases while processing the attribute value. 
 Note that we’ve limited the location of the value part of the 
attribute to accommodate startup performance requirements.  It’s not 
without precedent as at least one other attribute is also limited in 
amount of whitespace surrounding the value.


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
webrev: 
http://cr.openjdk.java.net/~sdrach/8153213/webrev.01/index.html 





this looks good to me. Good catch to delete the new jars added by the test.

I'll sponsor this for you once tests look OK.

Thanks!

/Claes



Thanks
Steve

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

[Steve Drach]

Hi,

I’ve updated the following patch, incorporating code by Claes Redestad to 
handle some corner cases while processing the attribute value.  Note that we’ve 
limited the location of the value part of the attribute to accommodate startup 
performance requirements.  It’s not without precedent as at least one other 
attribute is also limited in amount of whitespace surrounding the value.

> 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 

webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev.01/index.html 



Thanks
Steve

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

[Claes Redestad]

On 2016-04-10 06:36, Claes Redestad wrote:
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;
} 


Scratch that, this would fail to check for \r\n followed by space, thus 
incomplete.


/Claes

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

[Claes Redestad]

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, 
MULTIRELEASE_LASTOCC);

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;
}

Thanks!

/Claes

[1] 
http://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#Name-Value_pairs_and_Sections


On 2016-04-08 21:23, Steve Drach wrote:
The new webrev is 
http://cr.openjdk.java.net/~sdrach/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 > 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 
mailto:claes.redes...@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 fi

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

[Steve Drach]

The new webrev is http://cr.openjdk.java.net/~sdrach/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  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  
>> 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 | _]
> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
> 
> 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 
 
 webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
 
 Thanks
 Steve
> 

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

[Steve Drach]

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  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 | _]
 EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
 
 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 
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>> 
>>> Thanks
>>> Steve

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

[Claes Redestad]

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 | _]
EXTALPHANUM:=[ALPHA | 0-9 | SPACE]

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 

webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 

Thanks
Steve

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

[Steve Drach]

 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.

> 
> 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 | _]
>> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
>> 
>> 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 
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
> 
> Thanks
> Steve
>>> 
>> 
> 

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

[Paul Sandoz]

> On 8 Apr 2016, at 18:54, 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).

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 “otherchar”.


>> 
>> 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?

The value is literally the string “ true”, at least in one interpretation of 
the “spec” :-)

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)

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 | _]
> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
> 
> 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 
 
 webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
 
 Thanks
 Steve
>> 
> 

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

[Steve Drach]

>> 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.

> 
> 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.

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 | _]
EXTALPHANUM:=[ALPHA | 0-9 | SPACE]

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 
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>> 
>>> Thanks
>>> Steve
> 

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

[Paul Sandoz]

Hi Steve,

> On 8 Apr 2016, at 03:57, 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.

For example, the Sealed attribute can take on a value of “true” or “false” 
(case ignored), but there is no white space trimming.

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 
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>> 
>> Thanks
>> Steve

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

[Steve Drach]

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.

> 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 
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
> 
> Thanks
> Steve

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

[Claes Redestad]

Hi Steve,

this looks good to me!

Thanks!

/Claes

On 2016-04-01 22:37, Steve Drach wrote:

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 

webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 


Thanks
Steve

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

[Steve Drach]

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 

webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 


Thanks
Steve