RFR: JDK-8222489: jcmd VM.system_properties gives unusable paths on Windows
suenaga at oss.nttdata.com
Thu Feb 20 13:39:07 UTC 2020
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:
OTOH we can keep current behavior, we can implement more simply as below:
(It's similar to yours.)
> 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)` .
> `Properties::store(Writer)` says that `#`, `!`, `=`, `:` are written with a preceding backslash .
> So I think we should not use `Properties::store` to serialize properties.
>  https://download.java.net/java/early_access/jdk15/docs/api/java.base/java/util/Properties.html#store(java.io.OutputStream,java.lang.String)
>  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
More information about the serviceability-dev