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.k...@paratix.ch <mailto:philipp.k...@paratix.ch>
--
Gruss Philipp
------------------------------------------------------------------------
Paratix GmbH
St Peterhofstatt 11
8001 Zürich
+41 (0)76 397 79 35
philipp.k...@paratix.ch <mailto:philipp.k...@paratix.ch>