<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hello Vincent<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
Regards,<br>
Philipp<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 01.09.2017 15:20, Vincent Ryan
wrote:<br>
</div>
<blockquote
cite="mid:843FC180-8128-4638-97B5-DEBA3C9E6FDC@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
That all sounds fine. Let me know when your patch is ready to
submit.
<div class="">Thanks.<br class="">
<div class=""><br class="">
</div>
<div class=""><br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 1 Sep 2017, at 13:15, Philipp Kunz <<a
moz-do-not-send="true"
href="mailto:philipp.kunz@paratix.ch" class="">philipp.kunz@paratix.ch</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> Hello
Vincent<br class="">
<br class="">
Thank you for sponsoring!<br class="">
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: <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://www.oracle.com/technetwork/community/oca-486395.html#p">http://www.oracle.com/technetwork/community/oca-486395.html#p</a>
-> Paratix GmbH<br class="">
Therefore I think I have followed the steps in <a
moz-do-not-send="true"
href="http://openjdk.java.net/contribute/" class="">http://openjdk.java.net/contribute/</a>
at least the ones before actual patch submission.<br
class="">
<br class="">
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:<br class="">
make run-test-tier1<br class="">
make run-test TEST="jdk/test"<br class="">
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<br
class="">
make run-test
TEST="jtreg:jdk/test/sun/security/tools/jarsigner
jtreg:jdk/test/java/util/jar"<br class="">
then it reports zero errors and failures which may be
a good starting point.<br class="">
Do you think that sounds reasonable or do you have
another suggestion how to run the tests?<br class="">
<br class="">
Next, I will add a test for JDK-6695402 before
actually fixing it. As an example, I'll try something
in the style of
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk9/dev/jdk/file/65464a307408/test/java/util/jar/Manifest/CreateManifest.java">http://hg.openjdk.java.net/jdk9/dev/jdk/file/65464a307408/test/java/util/jar/Manifest/CreateManifest.java</a>.
This way, I try to demonstrate the improvement.<br
class="">
<br class="">
I guess I have identified the following line as the
cause: <span id="l157" class="">value = new
String(vb, 0, 0, vb.length);<br class="">
</span><a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/51f5d60713b5/src/java.base/share/classes/java/util/jar/Manifest.java#l157">http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/51f5d60713b5/src/java.base/share/classes/java/util/jar/Manifest.java#l157</a><br
class="">
So I'll try to remove it first including the whole
four line if block.<br class="">
<br class="">
Philipp<br class="">
<br class="">
<br class="">
<div class="moz-cite-prefix">On 01.09.2017 10:00,
Vincent Ryan wrote:<br class="">
</div>
<blockquote
cite="mid:2BE6B2AD-1896-4F22-A948-D494BA11F33A@oracle.com"
type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8" class="">
<div class="">Hello Philipp,
<div class=""><br class="">
</div>
<div class="">I’m happy to sponsor your fix for
JDK 10. Have you followed these steps: <a
moz-do-not-send="true"
href="http://openjdk.java.net/contribute/"
class="">http://openjdk.java.net/contribute/</a> ?</div>
<div class=""><br class="">
</div>
<div class="">Thanks.</div>
</div>
<div class=""><br class="">
</div>
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On 1 Sep 2017, at 08:58, Vincent
Ryan <<a moz-do-not-send="true"
href="mailto:vincent.x.ryan@oracle.com"
class="">vincent.x.ryan@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">Moved to security-dev<br
class="">
<br class="">
<br class="">
<blockquote type="cite" class="">On 1 Sep
2017, at 08:28, Philipp Kunz <<a
moz-do-not-send="true"
href="mailto:philipp.kunz@paratix.ch"
class="">philipp.kunz@paratix.ch</a>>
wrote:<br class="">
<br class="">
Hello everyone<br class="">
<br class="">
I have been developing with Java for
around 17 years now and when I encountered
some bug I decided to attempt to fix it: <a
moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-6695402"
class="">https://bugs.openjdk.java.net/browse/JDK-6695402</a>.
This also looks like it may not be too big
a piece for a first contribution.<br
class="">
<br class="">
I read through quite some guides and all
kinds of documents but could not yet help
myself with the following questions:<br
class="">
<br class="">
May I login to jira to add comments to
bugs? If so, how would I request or
receive credentials? Or are mailing lists
preferred?<br class="">
<br class="">
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.<br class="">
<br class="">
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.<br
class="">
<br class="">
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.<br
class="">
<br class="">
Philipp Kunz<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<title></title>
<br>
<br>
Gruss Philipp<br>
<br>
<br>
<br>
<hr size="2" width="100%"><br>
<img shrinktofit="true"
src="cid:part10.7B6C5393.4283A6F5@paratix.ch" align="bottom"
border="0"><br>
<br>
Paratix GmbH<br>
St Peterhofstatt 11<br>
8001 Zürich<br>
<br>
+41 (0)76 397 79 35<br>
<a href="mailto:philipp.kunz@paratix.ch">philipp.kunz@paratix.ch</a>
</div>
</body>
</html>