On 2014-02-23 01:19, Alan Bateman wrote:
On 19/02/2014 18:22, Mikael Vidstedt wrote:
:
The documented grammar in the comment only mentions "SPACE" and the
code below doesn't make any references to \t. As a matter of fact, it
only checks for one single, mandatory SPACE after the colon (enforced
at line 535-536) and doesn't care to remove any space characters at
the end of the value. The while loop only deals with continuations.
If additional spaces do exist they will as far as I can tell be part
of the value. Are they trimmed later? I'm assuming it would be nice
to have both parsers (parse_manifest & JarFacade) behave the same way?
Here's what it would look like to only check for space, but still eat
any additional spaces which doesn't match what
parse_manifest/parse_nv_pair does:
http://cr.openjdk.java.net/~mikael/webrevs/isspace/webrev.01/webrev/
Sorry for the delay getting back to you on this (I've been busy with
other things).
I checked the JAR File Specification, which is turn references RFC 822
as the "inspiration" for the name-value pairs. The SPACE token is just
ASCII SP. So I agree it's just ASCII SP that needs to be handled here,
not LWSP-char which includes ASCII HT.
Looking at JDK-6274276 then the trimming was done to avoid
hard-to-diagnose problems with leading/trailing spaces. It's possible
that this is inconsistent with other areas where JAR file attributes
are used. I would suggest leaving it as is for now as this is
potentially changing behavior in several areas.
That sounds reasonable. I'll keep the webrev.01 approach - only check
for and trim ASCII SP.
Thanks for the review!
Cheers,
Mikael
