Hello Vincent
I narrowed the error down so far and suspect now that it is an effect of
sun.security.util.ManifestDigester's constructor, lines 134 and 163-164,
in combination with java.util.jar.Manifest.make72Safe. Manifest breaks
multi-byte characters in manifest section names across lines and
ManifestDigester fails to restore them correctly, which both sounds
wrong but ManifestDigester sure is.
Just fixing it would be too tempting but then I would not know (I mean
not know for sure with evidence from tests) if any existing
jar-signature would still be valid and I don't want to break existing
signatures. When I had a look at the tests specific for Manifest and
ManifestDigester I found very little. There are many more different
situations and corner cases and combinations thereof to cover with tests
than it looks like at first glance so that writing tests that cover all
relevant cases looks to me like quite an effort. I have the impression
that test coverage was not a priority when the subjected code was
developed or at least the tests might not have been contributed to the
open JDK project. Hence, while I'm continuing to complete these tests I
miss, if someone has an idea how to simplify that so that I can still
convince at least myself that no existing signature will break or where
possibly more testcases are that I haven't discovered so far, please let
me know.
I'm also wondering how much performance should be taken into
consideration. There are a few hints such as reusing byte arrays that
suggest that it is an issue. Will a patch be rejected if it slows down
some tests beyond a certain limit for so little added value or what
might be the acceptance criteria here? I don't expect insuperable
difficulties with performance but would still appreciate some general idea.
My desired or currently preferred approach to fix JDK-6695402 would be
to let ManifestDigester any kind of use or extend Manifest in order to
let Manifest identify the manifest sections thereby preventing the
parsing duplicated that identifies the manifest sections.
As a new contributor I could use and would appreciate some guidance
particularly about what kind and completeness of test coverage and
performance tuning applies or is suggested in the context of the given
bug. Otherwise I fear I might contribute too many tests which would have
to be reviewed and maintained or quite some work would be for nothing if
a patch would not satisfy performance requirements.
Regards,
Philipp
On 01.09.2017 15:20, Vincent Ryan wrote:
That all sounds fine. Let me know when your patch is ready to submit.
Thanks.
On 1 Sep 2017, at 13:15, Philipp Kunz <philipp.k...@paratix.ch
<mailto:philipp.k...@paratix.ch>> wrote:
Hello Vincent
Thank you for sponsoring!
So far, I have become a contributor by signing the OCA which has been
accepted. Dalibor Topic wrote that he can confirm and it's also here:
http://www.oracle.com/technetwork/community/oca-486395.html#p ->
Paratix GmbH
Therefore I think I have followed the steps in
http://openjdk.java.net/contribute/ at least the ones before actual
patch submission.
Currently, I'm working on getting the existing test cases running
locally. Unfortunately, I started with jdk 9 and switched to 10 now.
I figured these commands run at least the relevant tests with 9 and
hope this also applies to 10:
make run-test-tier1
make run-test TEST="jdk/test"
they report errors and failures but it hasn't been completed and
released which might explain it. If I run the tests I assume relevant
for my patch
make run-test TEST="jtreg:jdk/test/sun/security/tools/jarsigner
jtreg:jdk/test/java/util/jar"
then it reports zero errors and failures which may be a good starting
point.
Do you think that sounds reasonable or do you have another suggestion
how to run the tests?
Next, I will add a test for JDK-6695402 before actually fixing it. As
an example, I'll try something in the style of
http://hg.openjdk.java.net/jdk9/dev/jdk/file/65464a307408/test/java/util/jar/Manifest/CreateManifest.java.
This way, I try to demonstrate the improvement.
I guess I have identified the following line as the cause: value =
new String(vb, 0, 0, vb.length);
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/51f5d60713b5/src/java.base/share/classes/java/util/jar/Manifest.java#l157
So I'll try to remove it first including the whole four line if block.
Philipp
On 01.09.2017 10:00, Vincent Ryan wrote:
Hello Philipp,
I’m happy to sponsor your fix for JDK 10. Have you followed these
steps: http://openjdk.java.net/contribute/ ?
Thanks.
On 1 Sep 2017, at 08:58, Vincent Ryan <vincent.x.r...@oracle.com
<mailto:vincent.x.r...@oracle.com>> wrote:
Moved to security-dev
On 1 Sep 2017, at 08:28, Philipp Kunz <philipp.k...@paratix.ch
<mailto:philipp.k...@paratix.ch>> wrote:
Hello everyone
I have been developing with Java for around 17 years now and when
I encountered some bug I decided to attempt to fix it:
https://bugs.openjdk.java.net/browse/JDK-6695402. This also looks
like it may not be too big a piece for a first contribution.
I read through quite some guides and all kinds of documents but
could not yet help myself with the following questions:
May I login to jira to add comments to bugs? If so, how would I
request or receive credentials? Or are mailing lists preferred?
Another question is whether I should apply it to jdk9, but it may
be too late now, or to jdk10, and backporting can be considered
later. Probably it wouldn't even make much a difference for the
patch itself.
One more question I have is how or where to find the sources from
before migration to mercurial. Because some lines of code I intend
to change go back farther and in the history I find only 'initial
commit'. With such a history I might be able better to understand
why it's there and prevent to make the same mistake again.
I guess the appropriate mailing list for above mentioned bug is
security-dev. Is it correct that I can send a patch there and just
hope for some sponsor to pick it up? Of course I'd be glad if some
sponsor would contact me and maybe provide some assistance or if
someone would confirm that sending a patch to the mailing list is
the right way to find a sponsor.
Philipp Kunz
--
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>
