これは何？

今回、パッチを書いたのはJDK-8222489のjcmd VM.system_properties gives unusable paths on Windows。

[JDK-8222489] jcmd VM.system_properties gives unusable paths on Windows - Java Bug System

Java 14までにはjcmd <PID> VM.system_propertiesを実行するとURLやWindowsのファイルパスに含まれる:や\と言う出力がエスケープされてhttps\://となったりC\:\\となってしまうよというバグ。タイトルだけみるとon Windowsとあるけど、文字列が該当すればプラットフォーム関係無しに発生します。

どうやって原因を特定したの？

まずは、ソースコード内をVM.system_propertiesで検索しました。すると、hotspot/share/services/diagnosticCommand.hppのPrintSystemPropertiesDCmdクラスが引っかかります。このクラスはexecuteメソッドぐらいしか怪しいところがないのでこれを見てみます。次に、hotspot/share/services/diagnosticCommand.cppでPrintSystemPropertiesDCmdクラスのexecuteの実装を見るとVMSupportクラスのserializePropertiesToByteArrayメソッドを呼んでいました。呼び出すぐらいしか処理をしていないのでこの中を追っていくと、文字列をbyte配列で取得している部分があり、その配列を見てみると\が足されていたのでこのあたりが怪しいなと思い絞り込んでいきました。

どんな実装だったのか

VMSupportクラスserializePropertiesToByteArrayメソッドの元々の実装はこんな感じ。

    private static byte[] serializePropertiesToByteArray(Properties p) throws IOException {
        ByteArrayOutputStream out = new ByteArrayOutputStream(4096);

        Properties props = new Properties();

        // stringPropertyNames() returns a snapshot of the property keys
        Set<String> keyset = p.stringPropertyNames();
        for (String key : keyset) {
            String value = p.getProperty(key);
            props.put(key, value);
        }

        props.store(out, null);
        return out.toByteArray();
    }

props.store(out, null);の中でエスケープする処理があるのだけど、おなじくloadメソッドで読み込めるような形で出力されるため、すこし余分にエスケープされてしまうのが原因。

どう直したの？

最初は、この実装をあまり変えずに問題となってる特殊文字だけどうにかしようとしたら、レビュアからNG。

好みの問題かもしれませんが、たぶん今の「やってから不要なものを引く」より「最初から望んでいるものを作る」方が理解しやすい気がするんですが…　ちょっと考えてみたんですが、制御文字の扱いをスマートにできれば、たぶん今よりコンパクトにできるかなと思ってます（まだ確証はない）
— Yasumasa Suenaga (@YaSuenag) 2020年2月20日

元の実装を参考にがっつりと作ることになった…最初はマルチバイト文字をCode Pointに変換すればいいかぐらいで考えてたので、codePointAtを使っていたら、このメソッドはcodePointを良い感じに補足してくれるので値が変わると言うことでNG。知らんかった…

replaceAll、これで動きました？codePointAtだとサロゲートペアをよしなに扱いすぎ（オリジナルとズレる）じゃないですか？

面倒ですが、続きはMLでしませんか？言うだけもアレなんで、実は私も2パターン作ったんですよ。正直あんまいい案じゃないかもだけど、参考と言うか議論には乗るかと思って。
— Yasumasa Suenaga (@YaSuenag) 2020年2月20日

しかも、Docには、Write the given properties list to a byte array and return it. Properties with a key or value that is not a String is filtered out. The stream written to the byte array is ISO 8859-1 encoded.とか書いてあるのに、元の実装では、ISO 8859-1 なんてまったく考慮されていなかったので、そのあたりを考慮することに。

以下のとおりCharsetEncoder と CoderResult を使えばできるよと教えて貰うものの、文字コードに関する処理なんてなったことが無くて涙目。

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/

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

とりあえず、エスケープと文字コード変換を分けて実装した。人生で初めてString(byte[], int, int)なんて使ったよ…

--- old/src/java.base/share/classes/jdk/internal/vm/VMSupport.java   2020-02-23 00:32:35.329976630 +0900
+++ new/src/java.base/share/classes/jdk/internal/vm/VMSupport.java    2020-02-23 00:32:35.165973461 +0900
@@ -26,6 +26,15 @@
 // 略
@@ -57,20 +66,53 @@
      */
     private static byte[] serializePropertiesToByteArray(Properties p) throws IOException {
         ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
+        PrintWriter bw = new PrintWriter(new OutputStreamWriter(out, "8859_1"));
 
-        Properties props = new Properties();
+        bw.println("#" + new Date().toString());
 
-        // stringPropertyNames() returns a snapshot of the property keys
-        Set<String> keyset = p.stringPropertyNames();
-        for (String key : keyset) {
-            String value = p.getProperty(key);
-            props.put(key, value);
+        try {
+            for (String key : p.stringPropertyNames()) {
+                String val = p.getProperty(key);
+                key = toISO88591(toEscapeSpecialChar(toEscapeSpace(key)));
+                val = toISO88591(toEscapeSpecialChar(val));
+                bw.println(key + "=" + val);
+            }
+        } catch (CharacterCodingException e) {
+            throw new RuntimeException(e);
         }
+        bw.flush();
 
-        props.store(out, null);
         return out.toByteArray();
     }
 
+    private static String toEscapeSpecialChar(String theString) {
+        return theString.replace("\t", "\\t").replace("\n", "\\n").replace("\r", "\\r").replace("\f", "\\f");
+    }
+
+    private static String toEscapeSpace(String source) {
+        return source.replace(" ", "\\ ");
+    }
+
+    private static String toISO88591(String source) throws CharacterCodingException {
+        var charBuf = CharBuffer.wrap(source);
+        var byteBuf = ByteBuffer.allocate(charBuf.length() * 6); // 6 is 2 bytes for '\\u' as String and 4 bytes for code point.
+        var encoder = StandardCharsets.ISO_8859_1
+                .newEncoder()
+                .onUnmappableCharacter(CodingErrorAction.REPORT);
+
+        CoderResult result;
+        do {
+            result = encoder.encode(charBuf, byteBuf, false);
+            if (result.isUnmappable()) {
+                byteBuf.put(String.format("\\u%04X", (int) charBuf.get()).getBytes());
+            } else if (result.isError()) {
+                result.throwException();
+            }
+        } while (result.isError());
+
+        return new String(byteBuf.array(), 0, byteBuf.position());
+    }
+
     public static byte[] serializePropertiesToByteArray() throws IOException {
         return serializePropertiesToByteArray(System.getProperties());
     }
--- /dev/null 2020-02-18 20:01:31.201608066 +0900
+++ new/test/jdk/sun/tools/jcmd/TestVM.java   2020-02-23 00:32:35.489979723 +0900
@@ -0,0 +1,59 @@
//略
+
+/*
+ * @test
+ * @bug 8222489
+ * @summary Unit test for jcmd utility. The test will send different diagnostic
+ * command requests to the current java process.
+ *
+ * @library /test/lib
+ *
+ * @run main/othervm TestVM
+ */
+public class TestVM {
+
+    public static void main(String[] args) throws Exception {
+        System.setProperty("normal", "normal_val");
+        System.setProperty("nonascii", "\u3042\u3044\u3046\u3048\u304A");
+        System.setProperty("url", "http://openjdk.java.net/");
+        System.setProperty("winDir", "C:\\");
+        System.setProperty("mix", "a\u3044u\u3048\u304Aka");
+        System.setProperty("space space", "blank blank");
+
+        OutputAnalyzer output = JcmdBase.jcmd("VM.system_properties");
+        output.shouldNotContain("https\\:");
+        output.shouldNotContain("C\\:\\\\");
+
+        output.shouldContain("normal=normal_val");
+        output.shouldContain("nonascii=\\u3042\\u3044\\u3046\\u3048\\u304A");
+        output.shouldContain("url=http://openjdk.java.net/");
+        output.shouldContain("winDir=C:\\");
+        output.shouldContain("mix=a\\u3044u\\u3048\\u304Aka");
+        output.shouldContain("space\\ space=blank blank");
+        output.shouldContain(Platform.isWindows() ? "line.separator=\\r\\n" : "line.separator=\\n");
+    }
+}