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