Thank you Henry. Feel free to modify it. Regards, Bo
> On 27 Sep 2018, at 09:39, Henry Jen <henry....@oracle.com> wrote: > > Thanks for the fix. I had looked into it, it’s a nice catch, glad you find it. > > The patch looks good, and test is sufficient, I have tested it with jdk.jdk > repo. I will help you to push the fix. The only minor improvement I like to > see is to verify content after the escape character, which I took the liberty > to do it. Webrev coming soon. > > Cheers, > Henry > >> On Sep 26, 2018, at 4:03 PM, Bo Zhang <zhangbo...@gmail.com >> <mailto:zhangbo...@gmail.com>> wrote: >> >> Can anyone please take a look? >> >> Thank you. >> >>> On 25 Sep 2018, at 14:27, Bo Zhang <zhangbo...@gmail.com >>> <mailto:zhangbo...@gmail.com>> wrote: >>> >>> Hi all, >>> >>> This is my first time to try to submit a patch. Please forgive me if I did >>> anything inappropriate. I’ve signed the OCA with the name “Bo Zhang”. >>> >>> Bug description: >>> >>> 8210810 <https://bugs.openjdk.java.net/browse/JDK-8210810 >>> <https://bugs.openjdk.java.net/browse/JDK-8210810>>: Incorrect argument >>> file parser behavior in certain cases >>> >>> Previously, if a quoted backslash character appears at argument file >>> parser buffer boundary (i.e. 4096th/8192nd/... character of an argument >>> file is backslash '\'), the segment before the backslash character will >>> be added into argument twice due to a missing anchor position reset >>> operation. >>> >>> For example, you have an argument file: >>> >>> -cp "...a\\b" >>> >>> and the 4096th and 4097th characters happen to be both '\', the result >>> generated by the argument file parser would be: >>> >>> -cp "...aa\\b” >>> >>> This patch fixes the issue by resetting anchor position correctly >>> in this case. >>> >>> I’m a little surprised that this issue has existed for several years >>> unnoticed. >>> >>> >>> Patch: >>> >>> # HG changeset patch >>> # User Bo Zhang <zhangbo...@gmail.com <mailto:zhangbo...@gmail.com> >>> <mailto:zhangbo...@gmail.com <mailto:zhangbo...@gmail.com>>> >>> # Date 1537835887 -28800 >>> # Tue Sep 25 08:38:07 2018 +0800 >>> # Node ID 34f2803ad62cde8c82e3dfcf0b66e80276bd7352 >>> # Parent 16f0deae8fa6ef85f7230308e1fe7556bd007c39 >>> 8210810: Incorrect argument file parser behavior in certain cases >>> >>> Previously, if a quoted backslash character appears at argument file >>> parser buffer boundary (i.e. 4096th/8192nd/... character of an argument >>> file is backslash '\'), the segment before the backslash character will >>> be added into argument twice due to a missing anchor position reset >>> operation. >>> >>> For example, you have an argument file: >>> >>> -cp "...a\\b" >>> >>> and the 4096th and 4097th characters happen to be both '\', the result >>> generated by the argument file parser would be: >>> >>> -cp "...aa\\b" >>> >>> This patch fixes the issue by resetting anchor position correctly >>> in this case. >>> >>> diff --git a/src/java.base/share/native/libjli/args.c >>> b/src/java.base/share/native/libjli/args.c >>> --- a/src/java.base/share/native/libjli/args.c >>> +++ b/src/java.base/share/native/libjli/args.c >>> @@ -262,6 +262,8 @@ >>> continue; >>> } >>> JLI_List_addSubstring(pctx->parts, anchor, nextc - anchor); >>> + // anchor after backslash character >>> + anchor = nextc + 1; >>> pctx->state = IN_ESCAPE; >>> break; >>> case '\'': >>> diff --git a/test/jdk/tools/launcher/Test8210810.java >>> b/test/jdk/tools/launcher/Test8210810.java >>> new file mode 100644 >>> --- /dev/null >>> +++ b/test/jdk/tools/launcher/Test8210810.java >>> @@ -0,0 +1,107 @@ >>> +/* >>> + * Copyright (c) 2018, 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 <http://www.oracle.com/> >>> <http://www.oracle.com/ <http://www.oracle.com/>> if you need additional >>> information or have any >>> + * questions. >>> + */ >>> + >>> +/* >>> + * @test >>> + * @bug 8210810 >>> + * @summary Incorrect argument file parser behavior when backslash appears >>> at buffer boundary >>> + * @modules java.base >>> + * @author Bo Zhang >>> + * @build TestHelper >>> + * @run main Test8210810 >>> + */ >>> + >>> +/* >>> + * 8210810: Previously, if a quoted backslash character appears at >>> argument file >>> + * parser buffer boundary (i.e. 4096th/8192nd/... character of an argument >>> + * file is backslash '\'), the segment before the backslash character will >>> + * be added into argument twice due to a missing anchor position reset >>> + * operation. This test creates a special argument file which can >>> reproduce this issue. >>> + */ >>> +import java.io.File; >>> +import java.nio.file.Files; >>> +import java.io.IOException; >>> +import java.util.Arrays; >>> + >>> +public class Test8210810 extends TestHelper { >>> + // Buffer size in args.c readArgFile() method >>> + private static final int ARG_FILE_PARSER_BUF_SIZE = 4096; >>> + public static void main(String... args) throws Exception { >>> + new Test8210810().canParseArgFileCorrectly(); >>> + } >>> + >>> + private void canParseArgFileCorrectly() throws IOException { >>> + File testJar = createTestClassWhichPrintsClasspath(); >>> + File argFile = createArgFileWithSpecificClasspath(); >>> + TestResult result = doExec(javaCmd, "@argFile", "Foo"); >>> + >>> + long zCountInArgFile = countZ(new >>> String(Files.readAllBytes(argFile.toPath()))); >>> + long zCountReadByParser = countZ(result.testOutput.get(0)); >>> + if(zCountInArgFile != zCountReadByParser) { >>> + throw new RuntimeException("Classpath parsed by parser is >>> different from argument file!"); >>> + } >>> + >>> + testJar.delete(); >>> + argFile.delete(); >>> + } >>> + >>> + private long countZ(String str) { >>> + return str.chars().filter(ch -> ch == 'z').count(); >>> + } >>> + >>> + private File createTestClassWhichPrintsClasspath() throws IOException { >>> + File testJar = new File("test.jar"); >>> + testJar.delete(); >>> + >>> + StringBuilder tsrc = new StringBuilder(); >>> + tsrc.append("public static void main(String... args) {\n"); >>> + tsrc.append(" System.out.println(\"Classpath:\" + >>> System.getProperty(\"java.class.path\"));\n"); >>> + tsrc.append("}\n"); >>> + createJar(testJar, new File("Foo"), tsrc.toString()); >>> + return testJar; >>> + } >>> + >>> + // Create a file whose 4096th and 4097th characters are both '\' >>> (backslash) >>> + // File content: >>> + // |-cp "test.jar:zzzzz...zzz\\"| >>> + private File createArgFileWithSpecificClasspath() throws IOException { >>> + File argFile = new File("argFile"); >>> + argFile.delete(); >>> + >>> + StringBuilder arg = new StringBuilder("-cp \"test.jar"); >>> + arg.append(File.pathSeparator); >>> + >>> + int fillingCharactersCount = ARG_FILE_PARSER_BUF_SIZE - 1 - >>> arg.length(); >>> + >>> + for(int i = 0; i < fillingCharactersCount; ++i) { >>> + arg.append('z'); >>> + } >>> + >>> + arg.append('\\'); // 4096th character in the arg file >>> + arg.append('\\'); // 4097th character in the arg file >>> + arg.append('"'); >>> + >>> + createAFile(argFile, Arrays.asList(arg.toString())); >>> + return argFile; >>> + } >>> +} >>> >>> Unit test: >>> >>> Included in the patch. It creates a special argument file whose 4096th and >>> 4097th characters are both ‘\’. Then it compares the classpath read in the >>> VM and the classpath in the argument file to verify they’re identical. >>> >>> <8210810.patch>
