Review Request: 6202130: java.util.jar.Attributes.writeMain() can't handle multi-byte chars

[Philipp Kunz]

In short, bug 6202130 is about removal of comments in Attributes like

> XXX Need to handle UTF8 values and break up lines longer than 72

The patch here contains a test that shows that utf characters are
handled correctly, which has been the case before. Breaking lines has
also been working before.

In the main code, the patch only removes a few comments and does not
change actual code. It all has been working before and now we know.

Philipp


https://bugs.openjdk.java.net/browse/JDK-6202130




On Wed, 2018-09-26 at 07:08 +0200, Philipp Kunz wrote:
> Hi,
> 
> There are some comments in the source code like this:
> 
>  > XXX Need to handle UTF8 values and break up lines longer than 72 bytes
> 
> in Attributes.java on lines 299, 327, and 370. Bug 6202130 also suggests 
> to add a test to show that the character set is fully supported before 
> actually removing those comments.
> 
> Hence, I added such a test, see ValueUtfEncoding in attached patch, and 
> it finds the complete utf character set supported correctly and that it 
> has been so already before.
> 
> The comments in Attributes also say that lines longer than 72 bytes need 
> to be broken. That is a different issue and has been already addressed 
> and solved in bug 6372077 and is now tested in LineBreakLineWidth and 
> therefore, that line break part of the comment is obsolete now as well 
> with the exception of version headers ("Manifest-Version" or 
> "Signature-Version") at least when taking strictly.
> 
> Version headers are not subject of bug 6202130 for which my patch 
> intends to provide a fix and if only because multi-byte characters are 
> not allowed in version headers and wasn't subject of 6372077 either 
> which was about too short lines rather than too long ones. If I would 
> change only the minimum I could argue was safe, there would have to 
> remain a comment in Attributes.java on line 327 like
> 
>  > XXX Need to handle version headers longer than 72 bytes
> 
> but I guess that should probably better be represented as another bug 
> rather than this style of comment if at all desired. There are some bugs 
> concerning version headers and particularly the behavior of the 
> manifests in their absence, among possibly others, 6910466, 4935610, 
> 4271239, 8196371 and 4256580 which have some relation but I haven't 
> found one that really fits the 72 byte line length limit of version 
> headers. Whether such a bug is created or found or not, it does not 
> directly relate to the one I'm trying to fix here, 6202130.
> 
> If also the test is good I would assume that the comments can be 
> removed now without actually changing the Manifest and Attributes 
> implementation.
> 
> Regards,
> Philipp
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6202130.patch
Type: text/x-patch
Size: 10222 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20181003/8cafdb7c/6202130-0001.patch>