Hi,

Was on vacation, I started to prepare the patch from webrev.04
for integration. Please note: made some adjustments to your
patch to pass jcheck, ie. usage of tabs and space at line endings,
and modifications to Copyright dates.

Also fixed a minor bug on unix replaced JLI_TRUE with JNI_TRUE.
I have attached a patch to for your reference.

However, there is a regression test failure on Windows,
jdk/test/tools/launcher/I18NTest.java

---Test info----
Executed command: C:\mmm\jdk\bin\javac.exe i18nH▒lloWorld.java

++++Test Output++++
javac: file not found: i18nHélloWorld.java
----End test info-----

Have you run all the launcher regression tests with this changeset ?

Thanks
Kumar

Hi Kumar, just wondering if there are any updates on processing this submission.
Thanks!

-----Original Message-----
From: Vladimir Shcherbakov
Sent: Wednesday, November 25, 2015 2:38 PM
To: Kumar Srinivasan <kumar.x.sriniva...@oracle.com>; Martin Sawicki 
<marc...@microsoft.com>
Cc: Kirk Shoop <kirk.sh...@microsoft.com>; core-libs-dev Libs 
<core-libs-dev@openjdk.java.net>
Subject: RE: RFR 8124977 cmdline encoding challenges on Windows

Hi Kumar,

Please find updated webreview here:
http://cr.openjdk.java.net/~kshoop/8124977/webrev.04/

Thanks,
Vladimir.

-----Original Message-----
From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com]
Sent: Sunday, November 22, 2015 8:14 AM
To: Martin Sawicki <marc...@microsoft.com>
Cc: Kirk Shoop <kirk.sh...@microsoft.com>; Vladimir Shcherbakov 
<vlas...@microsoft.com>; core-libs-dev Libs <core-libs-dev@openjdk.java.net>
Subject: Re: RFR 8124977 cmdline encoding challenges on Windows


Hi Martin, et. al.,

Sorry for not getting back earlier, I am very busy right now with my other 
large commitments for JDK9.

I will sponsor this "enhancement/bug fix" sometime in the new year, meanwhile, 
there is the changeset  [1] which is likely to cause merge conflicts, and perhaps logic 
issues.

Thanks
Kumar

[1]  
https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fhg.openjdk.java.net%2fjdk9%2fdev%2fjdk%2frev%2f3b201a9ef918&data=01%7c01%7cvlashch%40microsoft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=I2FKvBn82%2fxhW3D%2fi%2bRWaNOJk7Mg4lt2P0sdzLS%2fT9Q%3d
Hi all
Here's an updated webrev attempting to take into account the various pieces of 
feedback we have received:

Issue:
https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fbugs.
openjdk.java.net%2fbrowse%2fJDK-8124977&data=01%7c01%7cvlashch%40micro
soft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab2d7c
d011db47%7c1&sdata=FjmfM%2fnPbWB%2fMsUU8uDzAUo3aPu3zOELVsJO%2fsUIq9E%3
d
Webrev:
https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.openj
dk.java.net%2f~kshoop%2f8124977%2fwebrev.03%2f&data=01%7C01%7Cvlashch%
40microsoft.com%7C4d49ae546dba4d29b7be08d2f3589ee1%7C72f988bf86f141af9
1ab2d7cd011db47%7C1&sdata=101HBPar2AZ63GJWyubWH0DiKmNI%2bOxknN667BJnWY
0%3d

(Vladimir Shcherbakov is now working on this from our side)

Looking forward to any other feedback.
Thanks

-----Original Message-----
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
Behalf Of Kumar Srinivasan
Sent: Thursday, June 25, 2015 6:26 AM
To: Kirk Shoop (MS OPEN TECH) <kirk.sh...@microsoft.com>
Cc: Valery Kopylov (Akvelon) <v-val...@microsoft.com>; core-libs-dev
Libs <core-libs-dev@openjdk.java.net>
Subject: Re: RFR 8124977 cmdline encoding challenges on Windows

Hi Kirk,

Thanks for proposing this change.

If you notice all the posix calls are wrapped in JLI_* this  gives us the ability to use 
"W" functions.  I almost got it done, several years ago, but we upgraded to VS2010 and my 
work based on VS2003 keeled over, meanwhile my focus was  "shifted" to something else.

main.c: is really envisioned to be a stub  compiled by the tool
launchers, like java, javac, javah, jar etc. I prefer to see all the
heavy logic in this file moved to the platform specific file
windows/java_md.*

For the reason specified above we need to move fprintf or any naked posix calls 
to JLI_* indirections.

I don't see any tests ? The tests must be written in java and placed in 
jdk/test/tools/launcher, there is a helper framework TestHelper.java.

There are other changes in nio, charsets etc, this will be reviewed by my 
colleague specializing in that area (Sherman) cc'ed.


Thanks

Kumar






On 6/22/2015 2:01 PM, Kirk Shoop (MS OPEN TECH) wrote:
Hi,

Issue:
https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fbugs
.openjdk.java.net%2fbrowse%2fJDK-8124977&data=01%7c01%7cvlashch%40mic
rosoft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab2
d7cd011db47%7c1&sdata=FjmfM%2fnPbWB%2fMsUU8uDzAUo3aPu3zOELVsJO%2fsUIq
9E%3d

Webrev:
https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.open
jdk.java.net%2f~kshoop%2f8124977%2f&data=01%7C01%7Cvlashch%40microsof
t.com%7C4d49ae546dba4d29b7be08d2f3589ee1%7C72f988bf86f141af91ab2d7cd0
11db47%7C1&sdata=RAA%2b5aIzKtrk5X85oLXKlPzbpSk%2bgJZRI%2b0QSI11B0M%3d

This webrev intends to address interaction between Windows console and java 
apps.

Two switches were added that change the behavior of the launcher. The defaults 
do not change the launcher behavior.

     -Dwindows.UnicodeConsole=true - switches on Unicode support in the Windows 
console. This optional switch causes the launcher to call GetCommandLineW() and 
parse the arguments in unicode. It also modifies how the codepage for console 
output is selected.

     -Dfile.encoding.unicode="UTF-8" - identifies Unicode charset to use; If 
not specified, UTF-8 is used by default. Ignored when windows.UnicodeConsole is not set 
to true. When the first switch is used, this optional switch allows the codepage for 
console output to be controlled.

I would like to get feedback on the approach here and any additional work that 
is required solve these particular Unicode issues on Windows.

Kirk

diff --git a/make/lib/CoreLibraries.gmk b/make/lib/CoreLibraries.gmk
--- a/make/lib/CoreLibraries.gmk
+++ b/make/lib/CoreLibraries.gmk
@@ -370,6 +370,7 @@
         -export:JLI_CmdToArgs \
         -export:JLI_GetStdArgc \
         -export:JLI_GetStdArgs \
+        -export:JLI_DecodeArgs \
         -export:JLI_List_new \
         -export:JLI_List_add \
         -export:JLI_StringDup \
diff --git a/src/java.base/share/native/launcher/main.c b/src/java.base/share/native/launcher/main.c
--- a/src/java.base/share/native/launcher/main.c
+++ b/src/java.base/share/native/launcher/main.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -110,17 +110,9 @@
             }
         }
     }
-    JLI_CmdToArgs(GetCommandLine());
-    margc = JLI_GetStdArgc();
-    // add one more to mark the end
-    margv = (char **)JLI_MemAlloc((margc + 1) * (sizeof(char *)));
-    {
-        int i = 0;
-        StdArg *stdargs = JLI_GetStdArgs();
-        for (i = 0 ; i < margc ; i++) {
-            margv[i] = stdargs[i].arg;
-        }
-        margv[i] = NULL;
+
+    if (!JLI_DecodeArgs(&margc, &margv)) {
+        exit(1);
     }
 #else /* *NIXES */
     {
diff --git a/src/java.base/share/native/libjli/jli_util.h b/src/java.base/share/native/libjli/jli_util.h
--- a/src/java.base/share/native/libjli/jli_util.h
+++ b/src/java.base/share/native/libjli/jli_util.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -78,6 +78,7 @@
 #define JLI_StrNCaseCmp(p1, p2, p3)     strnicmp((p1), (p2), (p3))
 int  JLI_Snprintf(char *buffer, size_t size, const char *format, ...);
 void JLI_CmdToArgs(char *cmdline);
+jboolean JLI_DecodeArgs(int *pargc, char*** pargv);
 #define JLI_Lseek                       _lseeki64
 #define JLI_PutEnv                      _putenv
 #define JLI_GetPid                      _getpid
diff --git a/src/java.base/unix/native/libjli/java_md_common.c b/src/java.base/unix/native/libjli/java_md_common.c
--- a/src/java.base/unix/native/libjli/java_md_common.c
+++ b/src/java.base/unix/native/libjli/java_md_common.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -319,3 +319,8 @@
 {
     return NewPlatformStringArray(env, strv, argc);
 }
+
+jboolean
+JLI_DecodeArgs(int *pargc, char*** pargv) {
+    return JNI_TRUE;
+}
diff --git a/src/java.base/windows/native/libjava/Console_md.c b/src/java.base/windows/native/libjava/Console_md.c
--- a/src/java.base/windows/native/libjava/Console_md.c
+++ b/src/java.base/windows/native/libjava/Console_md.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -57,6 +57,8 @@
     int cp = GetConsoleCP();
     if (cp >= 874 && cp <= 950)
         sprintf(buf, "ms%d", cp);
+    else if (cp == 65001)
+        sprintf(buf, "UTF-8");
     else
         sprintf(buf, "cp%d", cp);
     return JNU_NewStringPlatform(env, buf);
diff --git a/src/java.base/windows/native/libjava/java_props_md.c b/src/java.base/windows/native/libjava/java_props_md.c
--- a/src/java.base/windows/native/libjava/java_props_md.c
+++ b/src/java.base/windows/native/libjava/java_props_md.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -145,6 +145,8 @@
     cp = GetConsoleCP();
     if (cp >= 874 && cp <= 950)
         sprintf(buf, "ms%d", cp);
+    else if (cp == 65001)
+        sprintf(buf, "UTF-8");
     else
         sprintf(buf, "cp%d", cp);
     return buf;
@@ -688,17 +690,27 @@
             }
 
             hStdOutErr = GetStdHandle(STD_OUTPUT_HANDLE);
-            if (hStdOutErr != INVALID_HANDLE_VALUE &&
-                GetFileType(hStdOutErr) == FILE_TYPE_CHAR) {
-                sprops.sun_stdout_encoding = getConsoleEncoding();
+            if (hStdOutErr != INVALID_HANDLE_VALUE) {
+                switch(GetFileType(hStdOutErr)) {
+                    case FILE_TYPE_DISK:
+                    case FILE_TYPE_CHAR:
+                    case FILE_TYPE_PIPE:
+                        sprops.sun_stdout_encoding = getConsoleEncoding();
+                        break;
+                }
             }
             hStdOutErr = GetStdHandle(STD_ERROR_HANDLE);
-            if (hStdOutErr != INVALID_HANDLE_VALUE &&
-                GetFileType(hStdOutErr) == FILE_TYPE_CHAR) {
-                if (sprops.sun_stdout_encoding != NULL)
-                    sprops.sun_stderr_encoding = sprops.sun_stdout_encoding;
-                else
-                    sprops.sun_stderr_encoding = getConsoleEncoding();
+            if (hStdOutErr != INVALID_HANDLE_VALUE) {
+                switch (GetFileType(hStdOutErr)) {
+                    case FILE_TYPE_DISK:
+                    case FILE_TYPE_CHAR:
+                    case FILE_TYPE_PIPE:
+                        if (sprops.sun_stdout_encoding != NULL)
+                            sprops.sun_stderr_encoding = sprops.sun_stdout_encoding;
+                        else
+                            sprops.sun_stderr_encoding = getConsoleEncoding();
+                        break;
+                }
             }
         }
     }
diff --git a/src/java.base/windows/native/libjli/java_md.c b/src/java.base/windows/native/libjli/java_md.c
--- a/src/java.base/windows/native/libjli/java_md.c
+++ b/src/java.base/windows/native/libjli/java_md.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1017,3 +1017,81 @@
     JLI_MemFree(filteredargs);
     return outArray;
 }
+
+/*
+ * Translate Unicode command line arguments to multi byte chars.
+ */
+static jboolean
+DecodeUnicodeArgs(LPWSTR cmd_line_wide, LPSTR* cmd_line_utf8) {
+    int cmd_line_utf8_length;
+
+    // First call to WideCharToMultiByte calculates destination buffer length.
+    cmd_line_utf8_length = WideCharToMultiByte(CP_UTF8,
+                                            0,
+                                            cmd_line_wide,
+                                            -1,
+                                            NULL,
+                                            0,
+                                            NULL,
+                                            NULL);
+    if (!cmd_line_utf8_length) {
+        JLI_ReportErrorMessage(
+            "WideCharToMultiByte failed to calculate destination buffer length "
+            "with error code %d", GetLastError());
+        return JNI_FALSE;
+    }
+
+    // Allocate buffer to receive conversion to UTF-8.
+    *cmd_line_utf8 = JLI_MemAlloc((size_t)cmd_line_utf8_length * sizeof(CHAR));
+
+    // Second call to WideCharToMultiByte does the actual conversion.
+    if (!WideCharToMultiByte(CP_UTF8,
+                             0,
+                             cmd_line_wide,
+                             -1,
+                             *cmd_line_utf8,
+                             cmd_line_utf8_length,
+                             NULL,
+                             NULL)) {
+        JLI_ReportErrorMessage(
+            "WideCharToMultiByte failed to convert to UTF-8 "
+            "with error code %d", GetLastError());
+        JLI_MemFree(*cmd_line_utf8);
+        *cmd_line_utf8 = NULL;
+        return JNI_FALSE;
+    }
+    return JNI_TRUE;
+}
+
+/*
+ * Translate command line arguments from Windows format to argc+argv.
+ */
+static void
+ConvertWinArgsToCommonFormat(LPSTR cmd_line, int *pargc, char*** pargv) {
+    JLI_CmdToArgs(cmd_line);
+
+    *pargc = JLI_GetStdArgc();
+    // add one more to mark the end
+    *pargv = (char **)JLI_MemAlloc((*pargc + 1) * (sizeof(char *)));
+    {
+        int i = 0;
+        StdArg *stdargs = JLI_GetStdArgs();
+        for (i = 0 ; i < *pargc ; ++i) {
+            (*pargv)[i] = stdargs[i].arg;
+        }
+        (*pargv)[i] = NULL;
+    }
+}
+
+jboolean
+JLI_DecodeArgs(int *pargc, char*** pargv) {
+    LPSTR cmdLineUtf8;
+
+    if (!DecodeUnicodeArgs(GetCommandLineW(), &cmdLineUtf8)) {
+        return JNI_FALSE;
+    }
+    ConvertWinArgsToCommonFormat(cmdLineUtf8, pargc, pargv);
+    JLI_MemFree(cmdLineUtf8);
+
+    return JNI_TRUE;
+}
diff --git a/test/tools/launcher/UnicodeCmdTest.java b/test/tools/launcher/UnicodeCmdTest.java
new file mode 100644
--- /dev/null
+++ b/test/tools/launcher/UnicodeCmdTest.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * This class ensures that the command line argument, passed as Unicode,
+ * matches the desired value.
+ *
+ * This is used by UnicodeCmdTestRun.java
+ */
+public class UnicodeCmdTest {
+    public static void main(String[] args) throws Exception {
+        if (args.length == 0 ) {
+            System.exit(1);
+        }
+
+        String desired = new String("\u042e\u043d\u0438\u043a\u043e\u0434");
+        if (args[0].equals(desired)) {
+            System.out.println("The argument matches the desired text: " + args[0]);
+        } else {
+            System.out.println("The argument does not match the desired text: " +
+                args[0] + " != " + desired);
+            System.exit(2);
+        }
+    }
+}
diff --git a/test/tools/launcher/UnicodeCmdTestRun.java b/test/tools/launcher/UnicodeCmdTestRun.java
new file mode 100644
--- /dev/null
+++ b/test/tools/launcher/UnicodeCmdTestRun.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8124977
+ * @summary Test 8124977 cmdline encoding challenges on Windows
+ * @compile UnicodeCmdTest.java
+ * @run main UnicodeCmdTestRun
+ */
+
+public class UnicodeCmdTestRun extends TestHelper {
+    public static void main(String... args) {
+        if (!isWindows) {
+            System.out.println("Test passes vacuously on non-windows");
+            return;
+        }
+        TestResult tr = doExec(javaCmd,
+            "-cp", TEST_CLASSES_DIR.getAbsolutePath(),
+            "-Dsun.jnu.encoding=\"UTF-8\"",
+            "UnicodeCmdTest", "\u042e\u043d\u0438\u043a\u043e\u0434");
+        System.out.println(tr.testOutput);
+        if (!tr.isOK()) {
+            throw new RuntimeException("Test failed");
+        }
+    }
+}

Reply via email to