[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: JDK-8222489: jcmd VM.system_properties gives unusable paths on Windows
From: Chihiro Ito <chiroito107 () gmail ! com>
Date: 2020-02-27 13:11:19
Message-ID: CAE_05uwttqkP06KStTEdQY_bbNTra4OHpQfszEcg5q_gbCxyrQ () mail ! gmail ! com
[Download RAW message or body]
Hi Ralf,
Thank you for your advice.
1.
The comment of serializePropertiesToByteArray in VMSupport is "The stream
written to the byte array is ISO 8859-1 encoded.".
But the previous implementation does not keep this. I think we need to
implement encode by ISO 8859-1.
2.
According to help, the feature of VM.system_properties is just "Print
system properties". The users should not use this output for loading. The
users use it when they want to see System Properties soon.
Regards,
Chihiro
2020年2月26日(水) 18:53 Schmelter, Ralf <ralf.schmelter@sap.com>:
> Hi Chihiro,
>
> I have two remarks:
>
> 1. ISO Latin 1 characters which are not ASCII will not work with the code.
> While the Properties.store() method claims to create ISO Latin 1 String, it
> really only will create printable ASCII characters (apart from the
> comment, but it is ASCII too in this case). See Properties.saveConvert,
> where the char is checked for < 0x20 or > 0x7e and then printed as \uxxxx.
> This is important, since the bytes of the ByteArrayOutputStream are then
> send to the jcmd. And jcmd expects UTF-8 encoded strings, which is OK if we
> only used ASCII characters. But a ISO Latin 1 character >= 0x80 will break
> the encoding. Just try using \u00DC in your test.
>
> 2. Your change makes it impossible to load the output with
> properties.load(). The old output could be loaded, since it was a valid
> properties file. But yours is not. For example, consider the filename
> c:\test\new. Formerly it would be encoded as:
> C\:\\test\\new
> And now it is:
> C:\test\new
> But the properties code would see "\n" as the newline character in your
> encoding. In fact you cannot differentiate between \n, \t, \f and \r
> originally being one or two characters.
>
> Best regards,
> Ralf
>
>
> From: serviceability-dev <serviceability-dev-bounces@openjdk.java.net> On
> Behalf Of Chihiro Ito
> Sent: Dienstag, 25. Februar 2020 04:45
> To: serguei.spitsyn@oracle.com
> Cc: serviceability-dev@openjdk.java.net
> Subject: Re: RFR: JDK-8222489: jcmd VM.system_properties gives unusable
> paths on Windows
>
> Hi Serguei,
>
> Thanks for your review and advice.
>
> I modified these.
> Could you review this again, please?
>
> Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.05/
>
> Regards,
> Chihiro
>
>
[Attachment #3 (text/html)]
<div dir="ltr">Hi Ralf,<div><br></div><div>Thank you for your \
advice.</div><div><br></div><div>1.</div><div>The comment of \
serializePropertiesToByteArray in VMSupport is "The stream written to the byte \
array is ISO 8859-1 encoded.".<br>But the previous implementation does not keep \
this. I think we need to implement encode by ISO 8859-1.<br><br>2.<br>According to \
help, the feature of VM.system_properties is just "Print system \
properties". The users should not use this output for loading. The users use it \
when they want to see System Properties soon. \
<br></div><div><br></div><div>Regards,</div><div>Chihiro</div><div><br></div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">2020年2月26日(水) 18:53 \
Schmelter, Ralf <<a \
href="mailto:ralf.schmelter@sap.com">ralf.schmelter@sap.com</a>>:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex">Hi Chihiro,<br> <br>
I have two remarks:<br>
<br>
1. ISO Latin 1 characters which are not ASCII will not work with the code. While the \
Properties.store() method claims to create ISO Latin 1 String, it really only will \
create printable ASCII characters (apart from the comment, but it is ASCII too in \
this case). See Properties.saveConvert, where the char is checked for < 0x20 or \
> 0x7e and then printed as \uxxxx. This is important, since the bytes of the \
ByteArrayOutputStream are then send to the jcmd. And jcmd expects UTF-8 encoded \
strings, which is OK if we only used ASCII characters. But a ISO Latin 1 character \
>= 0x80 will break the encoding. Just try using \u00DC in your test.<br> <br>
2. Your change makes it impossible to load the output with properties.load(). The old \
output could be loaded, since it was a valid properties file. But yours is not. For \
example, consider the filename c:\test\new. Formerly it would be encoded as:<br> \
C\:\\test\\new<br> And now it is:<br>
C:\test\new<br>
But the properties code would see "\n" as the newline character in your \
encoding. In fact you cannot differentiate between \n, \t, \f and \r originally being \
one or two characters.<br> <br>
Best regards,<br>
Ralf<br>
<br>
<br>
From: serviceability-dev <<a \
href="mailto:serviceability-dev-bounces@openjdk.java.net" \
target="_blank">serviceability-dev-bounces@openjdk.java.net</a>> On Behalf Of \
Chihiro Ito<br>
Sent: Dienstag, 25. Februar 2020 04:45<br>
To: <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br>
Cc: <a href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>
Subject: Re: RFR: JDK-8222489: jcmd VM.system_properties gives unusable paths on \
Windows<br> <br>
Hi Serguei,<br>
<br>
Thanks for your review and advice.<br>
<br>
I modified these. <br>
Could you review this again, please?<br>
<br>
Webrev : <a href="http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.05/" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.05/</a> \
<br> <br>
Regards,<br>
Chihiro<br>
<br>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic