Patch for JDK-6695402: Jarsigner with multi-byte characters in class names

[Philipp Kunz]

Hi Max

Thank you for your update and its associated effort.

I suggest that at least a comment should be added about why and how 
non-existing files can be added and the test still serves it's purpose. 
In fact I was quite a bit surprised when I found out that 
JarUtils.createJar adds the file name as contents if the file cannot be 
found. Ideally, we would also add some note about why this was relevant, 
about the test not compiling on certain oses with jtreg together with 
the acute but it doesn't apply any longer now. Altogether the test has 
become even simpler now.

One more small thing that I just noticed now is that I would prefer 
ManifestFileName not to start with a capital letter like the other 
nearby variables. I thought testName was too ambiguous a name and 
changed it to testClassName. I also reviewed and slightly changed a few 
comments again. Of course it will never become perfect but now it should 
do. See attached patch.

The contributed by is fine. I'd be glad to share credits with you and 
please accept more flattering for your collaboration.

Regards,
Philipp



On 27.09.2017 03:20, Weijun Wang wrote:
> Hi Philipp
>
> The problem is that when launching by jtreg javac has difficulties writing the class file with the é char into the file system on several OSes.
>
> I've updated the test a little. Now they are not written to files. Fortunately JarUtils can add non-existing entries. The test now passes on all our testing platforms.
>
>     http://cr.openjdk.java.net/~weijun/6695402/webrev.01
>
> If you are OK with the new version I'll push them.
>
> Thanks
> Max
>
>> On Sep 26, 2017, at 10:54 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>
>> It might be a jtreg issue, but I'll have to get it resolved before pushing your changeset.
>>
>> --Max
>>
>>> On Sep 26, 2017, at 7:30 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>>
>>> Oops, the new test fails on Linux and Solaris.
>>>
>>> /scratch/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java:54: error: error while writing A1234567890B1234567890C123456789D1?xyz: bad filename RelativeFile[LineBrokenMultiByteCharacter$A1234567890B1234567890C123456789D1?xyz.class]
>>>    static class A1234567890B1234567890C123456789D1\u00E9xyz { }
>>>           ^
>>> 1 error
>>>
>>> I'll ask the compiler team.
>>>
>>> --Max
>>>
>>>> On Sep 26, 2017, at 3:51 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>>>
>>>>
>>>>> On Sep 26, 2017, at 1:37 PM, Philipp Kunz <philipp.kunz at paratix.ch> wrote:
>>>>>
>>>>> Hi Max
>>>>>
>>>>> This time I got it with readAllBytes. Thank you for the hint.
>>>>>
>>>>> Apparently, UTF characters are allowed in source code, particularly in identifiers here, which also has caused the bug. Even if only for sending patches around I changed it and was surprised to see escaping working not only in strings but also in identifiers.
>>>> See https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.2
>>>>
>>>> I've submitted your change to our testing server. Once it's OK, I'll push the changeset.
>>>>
>>>> I assume "Contributed-by: Philipp Kunz <philipp.kunz at paratix.ch>" is good.
>>>>
>>>> BTW, there are several TAB chars and trailing spaces in your patch. I've removed them.
>>>>
>>>> Thanks for your contribution.
>>>>
>>>> --Max
>>>>
>>>>> When I had another look at the test I came to the conclusion that it does not need what has been named refClassFileName before. The purpose of the test is only to check a signature of a class with a     two byte character in its name and not at the same time to verify that if that test failed it is specifically because of the name. If it fails there is a problem no matter why. In the beginning it was handy to see the difference but I don't think it should be kept and maintained so I removed it. For the update signature case a second file to sign is still required though.
>>>>>
>>>>> I considered multi-byte a one word before but now I also prefer it with a capital b. Anyway, this name might not be the best choice and I changed it to LineBrokenMultiByteCharacter.
>>>>>
>>>>> See attached patch.
>>>>>
>>>>> Regards,
>>>>> Philipp
>>>>>

-- 


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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170927/07bc3bdf/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 5134 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170927/07bc3bdf/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: JDK-6695402.patch
Type: text/x-patch
Size: 12456 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170927/07bc3bdf/JDK-6695402.patch>