JDK-6372077: JarFile.getManifest() should handle manifest attribute names up to 70 bytes
Roger Riggs
Roger.Riggs at Oracle.com
Mon Dec 18 15:46:03 UTC 2017
Hi Phillip,
Sorry for the silence...
I/we haven't had time to full understand the ramifications of the change
you propose.
It seems there is a difficult/unresolvable conflict in the
specifications between the line length
requirements and the header specs.
Regards, Roger
On 11/21/2017 1:18 AM, Philipp Kunz wrote:
> Hi everyone,
>
> I haven't got any reply now for around three weeks and now i start to
> wonder if I just missed it or if I should refine my approach to find a
> sponsor or if it helped if I presented a ready patch or if noone
> considers this important enough or anything else whatever. This is
> only my second contribution hence I don't know the procedure well.
>
> One point maybe worth to point out again is that I don't want to
> support manifest headers longer than 70 character, just up to 70,
> which has always been the intention but has only worked up to 68. This
> might have been written confusingly in my last email.
>
> As far as I understood, I should first get a sponsor. In any case, is
> there any suggestion for how to proceed?
>
> Regards,
> Philipp
>
>
>
> On 03.11.2017 00:04, Philipp Kunz wrote:
>> Hi Sean and Max and all others,
>>
>> Thank you Sean for the hint about the right mailing list. And thanks
>> also for his hint to Max to make smaller portions of changes.
>>
>> I would like to contribute a fix for JDK-6372077 which is about
>> JarFile.getManifest() should handle manifest attribute name[s longer
>> than] 70 bytes.
>>
>> It looks like the bug is caused by Manifest.make72Safe breaking lines
>> at 70 bytes instead of 72 despite its comment and name
>> (http://hg.openjdk.java.net/jdk10/master/file/tip/src/java.base/share/classes/java/util/jar/Manifest.java#l176).The
>> resulting StringBuffer has then lines of 72 bytes maximum each
>> including the following line break. Without the line break that
>> leaves 70 bytes of characters per line. On the other hand, header
>> names can be up to 70 bytes (only single-byte utf-8 characters) and
>> cannot be broken across a line break and need to be followed by a
>> colon and a space which must be on the same line too according to the
>> specification. When breaking at 70 bytes excluding the line break,
>> however, long header names don't fit in one line together with the
>> colon space delimiter because there is not sufficient space.
>> Manifests with names up to 70 bytes long can still be written without
>> immediate exception but the resulting manifests are illegal in my
>> opinion. When later reading such manifests
>> (http://hg.openjdk.java.net/jdk10/master/file/tip/src/java.base/share/classes/java/util/jar/Attributes.java#l406),
>> an error occurs as a consequence of the bad manifest. This is more or
>> less the current situation and also what JDK-6372077 already knew.
>>
>> --> After all, in order to fix it, i'd like to propose to make
>> manifest file lines wider by two bytes.
>>
>> The only proper alternative that came into my mind would be to change
>> the manifest specification and reduce the maximum header name length
>> there by two and also in the code. If that would break any existing
>> code i guess that would affect code only that produced invalid
>> manifests and would be acceptable.
>>
>> Supporting all existing and possibly invalid manifests would mean to
>> add support for reading headers the delimiters of which are broken
>> onto the next line which I consider too complex with respect to the
>> value added and even more so considering that those invalid manifest
>> can be assumed to have been detected as such by reading them and also
>> because it would be a feature that would be used less and less over
>> time if the code to write manifest is changed at the same time to
>> produce only valid manifests in the discussed respect here. I don't
>> think this should be actually done.
>>
>> Before I actually do the leg work, i'd like to ask, if there are
>> concerns or objections or tips for such a change or if anyone can or
>> cannot follow the reasoning and the conclusion to make manifests 2
>> bytes wider or if i missed an important point altogether.
>>
>> In case there will be a consent about how to solve this, would
>> someone volunteer to sponsor? That may be less urgent at the moment
>> than the question above about how to proceed.
>>
>> Philipp
>>
>>
>> On 12.10.2017 22:32, Sean Mullan wrote:
>>> Hi Phillip,
>>>
>>> All of these bugs are in the core-libs area, so I am copying the
>>> core-libs-dev list since that is where they should be discussed and
>>> reviewed. I have -bcc-ed security-dev (where this was originally
>>> posted).
>>>
>>> --Sean
>>>
>>> On 10/2/17 1:24 PM, Philipp Kunz wrote:
>>>> Hi,
>>>>
>>>> While fixing JDK-6695402 I came across other related bugs to
>>>> manifests such as JDK-6372077, JDK-6202130, JDK-8176843,
>>>> JDK-4842483, and JDK-6443578 which all relate to manifest reading
>>>> and writing. Somewhere bug 7071055 is mentioned but I cannot find
>>>> anywhere. Another group of bugs, JDK-6910466, JDK-4935610, and
>>>> JDK-4271239 concern the mandatory manifest main attributes
>>>> Manifest-Version or Signature-Version and at first glance are
>>>> duplicates. If you know of more known bugs, not necessarily present
>>>> in jira, I'd be glad to get notified.
>>>>
>>>> There are also some comments about utf handling and line breaking
>>>> in the code of Manifest:
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Attributes.java#l299
>>>>
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Attributes.java#l327
>>>>
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Attributes.java#l370
>>>>
>>>>
>>>> Furthermore, Attributes.map should declare appropriate type
>>>> parameters:
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Attributes.java#l61
>>>>
>>>> The specification would also require that `header names must not
>>>> start with _, - or "From"`
>>>> (http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.html#Section-Specification)
>>>> but I would opt not to add this as a hard restriction because I
>>>> guess it can be assumed that such names are in use now after having
>>>> been working for years. A warning to a logger might be conceivable
>>>> such as in
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Attributes.java#l424
>>>>
>>>> Attribute values are never checked at all and invalid characters
>>>> such as line breaks are never detected except that when reading the
>>>> manifest again the values are cut off.
>>>> The tab character (U+0008) does not work in manifest values.
>>>> I suspect that there are also issues regarding the iteration order
>>>> but I haven't got a prove yet unlike for the other points above:
>>>> http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/share/classes/java/util/jar/Manifest.java#l54
>>>>
>>>> There is duplicated or very similar code in Attributes and
>>>> Manifest: Attributes.write-Manifest.write-Attributes.writeMain and
>>>> Attributes.read-Manifest.read.
>>>> Resolving JDK-6202130 would have the additional benefit to be able
>>>> to view manifests with any utf-8 capable editor even if multi-byte
>>>> characters break across lines.
>>>>
>>>> Fixing these issues individually looks like more complicated work
>>>> than fixing them all at once, I guess, also because of a very low
>>>> current test coverage. So I'd suggest to add some thorough tests
>>>> along with fixing these issues. But before I start I would like to
>>>> get some guidance, assistance or at least confirmation on how to
>>>> proceed. I'm new to open jdk and have submitted only one patch so far.
>>>>
>>>> Is it ok to add tests for things that have worked before?
>>>> Is it ok to refactor duplicated code just for the added value to
>>>> reduce effort for testing?
>>>> I assume compatibility to and from existing manifests is the
>>>> highest priority, correct? This would also be the hard part in
>>>> adding as complete test coverage as possible. What would be
>>>> acceptable criteria to meet?
>>>> Why does Attributes not extend LinkedHashMap and why does Manifest
>>>> not extend HashMap? Any objection?
>>>> While I would not want to write code that looks slow or change more
>>>> than necessary one can never know before having performance
>>>> actually measured. Is there some way this is dealt with or should I
>>>> wait for such feedback until after patch submission?
>>>>
>>>> Would someone sponsor?
>>>>
>>>> Regards,
>>>> Philipp
>>>>
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>>
>>>> Paratix GmbH
>>>> St Peterhofstatt 11
>>>> 8001 Zürich
>>>>
>>>> +41 (0)76 397 79 35
>>>> philipp.kunz at paratix.ch <mailto:philipp.kunz at paratix.ch>
>>
>> --
>>
>>
>> Gruss Philipp
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>>
>> Paratix GmbH
>> St Peterhofstatt 11
>> 8001 Zürich
>>
>> +41 (0)76 397 79 35
>> philipp.kunz at paratix.ch <mailto:philipp.kunz at paratix.ch>
>
More information about the core-libs-dev
mailing list