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 <[email protected]
<mailto:[email protected]>>:
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
