RFR: JDK-8222489: jcmd VM.system_properties gives unusable paths on Windows

[Chihiro Ito]

Hi Yasumasa,

Thank you for your advice.

I decided not to use regular expressions. because of the number of \is
confusing.
I stopped using codePointAt() and used CharsetEncoder to work with ISO 8859
-1.
I added some environment variables to the test. However, environment
variables that contain multi bytes or spaces are not included because jtreg
does not support them.

Could you review this again, please?

Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.02/

Regards,
Chihiro

2020年2月20日(木) 22:39 Yasumasa Suenaga <suenaga at oss.nttdata.com>:

> Hi Chihiro,
>
> On 2020/02/20 20:20, Chihiro Ito wrote:
> > Hi Yasumasa,
> >
> > Thank you for your quick review.
> >
> > I modified the code without Properties::store.
> >
> > Could you review this again, please?
> >
> > Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.01/
>
>    - Your change shows "\n" as "\\n". Is it ok? Currently "\n" would be
> shown straightly.
>    - Your change uses Character::codePointAt to convert char to int value.
>      According to Javadoc, it would be different value if a char is in
> surrogate range.
>    - Description of serializePropertiesToByteArray() says the return value
> is encoded in ISO 8859-1,
>      but it does not seems to be so because the logic depends on the spec
> of Properties::store. Is it ok?
>    - Test case does not stable because system properties might be
> different from your environment.
>      I suggest you to set system properties for testing explicitly. E.g.
>          -Dnormal=normal_val -D"space space=blank blank" -Dnonascii=あいうえお
> -Dopenjdk_url=http://openjdk.java.net/ -Dbackslash="\\"
>        * Also I recommend you to check "\n" in the test from
> `line.separator`. I think it is stable property.
>
> I've not convinced whether we should compliant to the comment which says
> for ISO 8859-1.
> If it is important, we can use CharsetEncoder from ISO_8859_1 as below:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8222489/proposal-encoder/
>
> OTOH we can keep current behavior, we can implement more simply as below:
> (It's similar to yours.)
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8222489/proposal-props-style/
>
>
> Thanks,
>
> Yasumasa
>
>
> > Regards,
> > Chihiro
> >
> >
> > 2020年2月20日(木) 9:34 Yasumasa Suenaga <suenaga at oss.nttdata.com <mailto:
> suenaga at oss.nttdata.com>>:
> >
> >     Hi Chihiro,
> >
> >     I think this problem is caused by spec of
> `Properties::store(Writer)`.
> >
> >     `Properties::store(OutputStream)` says that the output format is as
> same as `store(Writer)` [1].
> >     `Properties::store(Writer)` says that `#`, `!`, `=`, `:` are written
> with a preceding backslash [2].
> >
> >     So I think we should not use `Properties::store` to serialize
> properties.
> >
> >
> >     Thanks,
> >
> >     Yasumasa
> >
> >
> >     [1]
> https://download.java.net/java/early_access/jdk15/docs/api/java.base/java/util/Properties.html#store(java.io.OutputStream,java.lang.String)
> >     [2]
> https://download.java.net/java/early_access/jdk15/docs/api/java.base/java/util/Properties.html#store(java.io.Writer,java.lang.String)
> >
> >
> >     On 2020/02/19 22:36, Chihiro Ito wrote:
> >      > Hi,
> >      >
> >      > Could you review this tiny fix, please?
> >      >
> >      > This problem affected not the only path on Windows, but also
> Linux and URLs using ":".
> >      >
> >      > Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.00/
> >      > JBS : https://bugs.openjdk.java.net/browse/JDK-8222489
> >      >
> >      > Regards,
> >      > Chihiro
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200222/540123fe/attachment.htm>