8144220: UL does not support full path names for log files on windows
authormlarsson
Mon, 04 Jan 2016 11:31:42 +0100
changeset 35229 b8d78ee728b2
parent 35228 e7bbfb90fd31
child 35230 a528ea8203ec
8144220: UL does not support full path names for log files on windows Reviewed-by: sla, mgronlun
hotspot/src/share/vm/logging/logConfiguration.cpp
hotspot/src/share/vm/logging/logConfiguration.hpp
hotspot/test/serviceability/logging/TestQuotedLogOutputs.java
--- a/hotspot/src/share/vm/logging/logConfiguration.cpp	Mon Jan 04 11:37:18 2016 +0100
+++ b/hotspot/src/share/vm/logging/logConfiguration.cpp	Mon Jan 04 11:31:42 2016 +0100
@@ -110,7 +110,7 @@
   return SIZE_MAX;
 }
 
-LogOutput* LogConfiguration::new_output(char* name, const char* options) {
+LogOutput* LogConfiguration::new_output(char* name, const char* options, outputStream* errstream) {
   const char* type;
   char* equals_pos = strchr(name, '=');
   if (equals_pos == NULL) {
@@ -121,16 +121,34 @@
     name = equals_pos + 1;
   }
 
+  // Check if name is quoted, and if so, strip the quotes
+  char* quote = strchr(name, '"');
+  if (quote != NULL) {
+    char* end_quote = strchr(name + 1, '"');
+    if (end_quote == NULL) {
+      errstream->print_cr("Output name has opening quote but is missing a terminating quote.");
+      return NULL;
+    } else if (quote != name || end_quote[1] != '\0') {
+      errstream->print_cr("Output name can not be partially quoted."
+                          " Either surround the whole name with quotation marks,"
+                          " or do not use quotation marks at all.");
+      return NULL;
+    }
+    name++;
+    *end_quote = '\0';
+  }
+
   LogOutput* output;
   if (strcmp(type, "file") == 0) {
     output = new LogFileOutput(name);
   } else {
-    // unsupported log output type
+    errstream->print_cr("Unsupported log output type.");
     return NULL;
   }
 
   bool success = output->initialize(options);
   if (!success) {
+    errstream->print_cr("Initialization of output '%s' using options '%s' failed.", name, options);
     delete output;
     return NULL;
   }
@@ -274,32 +292,40 @@
   char* copy = os::strdup_check_oom(opts, mtLogging);
 
   // Split the option string to its colon separated components.
-  char* what = NULL;
-  char* output_str = NULL;
-  char* decorators_str = NULL;
-  char* output_options = NULL;
+  char* str = copy;
+  char* substrings[4] = {0};
+  for (int i = 0 ; i < 4; i++) {
+    substrings[i] = str;
 
-  what = copy;
-  char* colon = strchr(what, ':');
-  if (colon != NULL) {
-    *colon = '\0';
-    output_str = colon + 1;
-    colon = strchr(output_str, ':');
-    if (colon != NULL) {
-      *colon = '\0';
-      decorators_str = colon + 1;
-      colon = strchr(decorators_str, ':');
-      if (colon != NULL) {
-        *colon = '\0';
-        output_options = colon + 1;
+    // Find the next colon or quote
+    char* next = strpbrk(str, ":\"");
+    while (next != NULL && *next == '"') {
+      char* end_quote = strchr(next + 1, '"');
+      if (end_quote == NULL) {
+        log_error(logging)("Missing terminating quote in -Xlog option '%s'", str);
+        os::free(copy);
+        return false;
       }
+      // Keep searching after the quoted substring
+      next = strpbrk(end_quote + 1, ":\"");
+    }
+
+    if (next != NULL) {
+      *next = '\0';
+      str = next + 1;
+    } else {
+      break;
     }
   }
 
-  // Parse each argument
+  // Parse and apply the separated configuration options
+  char* what = substrings[0];
+  char* output = substrings[1];
+  char* decorators = substrings[2];
+  char* output_options = substrings[3];
   char errbuf[512];
   stringStream ss(errbuf, sizeof(errbuf));
-  bool success = parse_log_arguments(output_str, what, decorators_str, output_options, &ss);
+  bool success = parse_log_arguments(output, what, decorators, output_options, &ss);
   if (!success) {
     errbuf[strlen(errbuf) - 1] = '\0'; // Strip trailing newline.
     log_error(logging)("%s", errbuf);
@@ -340,14 +366,9 @@
     idx = find_output(outputstr);
     if (idx == SIZE_MAX) {
       char* tmp = os::strdup_check_oom(outputstr, mtLogging);
-      LogOutput* output = new_output(tmp, output_options);
+      LogOutput* output = new_output(tmp, output_options, errstream);
       os::free(tmp);
       if (output == NULL) {
-        errstream->print("Unable to add output '%s'", outputstr);
-        if (output_options != NULL && strlen(output_options) > 0) {
-          errstream->print(" with options '%s'", output_options);
-        }
-        errstream->cr();
         return false;
       }
       idx = add_output(output);
--- a/hotspot/src/share/vm/logging/logConfiguration.hpp	Mon Jan 04 11:37:18 2016 +0100
+++ b/hotspot/src/share/vm/logging/logConfiguration.hpp	Mon Jan 04 11:31:42 2016 +0100
@@ -43,7 +43,7 @@
   static bool         _post_initialized;
 
   // Create a new output. Returns NULL if failed.
-  static LogOutput* new_output(char* name, const char* options = NULL);
+  static LogOutput* new_output(char* name, const char* options, outputStream* errstream);
 
   // Add an output to the list of configured outputs. Returns the assigned index.
   static size_t add_output(LogOutput* out);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/serviceability/logging/TestQuotedLogOutputs.java	Mon Jan 04 11:31:42 2016 +0100
@@ -0,0 +1,109 @@
+/*
+ * Copyright (c) 2015, 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 TestQuotedLogOutputs
+ * @summary Ensure proper parsing of quoted output names for -Xlog arguments.
+ * @library /testlibrary
+ */
+
+import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.ProcessTools;
+import jdk.test.lib.OutputAnalyzer;
+
+public class TestQuotedLogOutputs {
+
+    public static void main(String[] args) throws Exception {
+        // Ensure log files can be specified with full path.
+        // On windows, this means that the file name will contain
+        // a colon ('C:\log.txt' for example), which is used to
+        // separate -Xlog: options (-Xlog:tags:filename:decorators).
+        // Try to log to a file in our current directory, using its absolute path.
+        String baseName = "test file.log";
+        Path filePath = Paths.get(baseName).toAbsolutePath();
+        String fileName = filePath.toString();
+        File file = filePath.toFile();
+
+        // In case the file already exists, attempt to delete it before running the test
+        file.delete();
+
+        // Depending on if we're on Windows or not the quotation marks must be escaped,
+        // otherwise they will be stripped from the command line arguments.
+        String quote;
+        if (System.getProperty("os.name").toLowerCase().contains("windows")) {
+            quote = "\\\""; // quote should be \" (escaped quote)
+        } else {
+            quote = "\""; // quote should be " (no escape needed)
+        }
+
+        // Test a few variations with valid log output specifiers
+        String[] validOutputs = new String[] {
+            quote + fileName + quote,
+            "file=" + quote + fileName + quote,
+            quote + fileName + quote + ":",
+            quote + fileName + quote + "::"
+        };
+        for (String logOutput : validOutputs) {
+            // Run with logging=trace on stdout so that we can verify the log configuration afterwards.
+            ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-Xlog:logging=trace",
+                                                                      "-Xlog:all=trace:" + logOutput,
+                                                                      "-version");
+            OutputAnalyzer output = new OutputAnalyzer(pb.start());
+            output.shouldHaveExitValue(0);
+            Asserts.assertTrue(file.exists());
+            file.deleteOnExit(); // Clean up after test
+            output.shouldMatch("\\[logging *\\].*" + baseName); // Expect to see the log output listed
+        }
+
+        // Test a bunch of invalid output specifications and ensure the VM fails with these
+        String[] invalidOutputs = new String[] {
+            quote,
+            quote + quote, // should fail because the VM will try to create a file without a name
+            quote + quote + quote,
+            quote + quote + quote + quote,
+            quote + quote + quote + quote + quote,
+            "prefix" + quote + quote + "suffix",
+            "prefix" + quote + quote,
+            quote + quote + "suffix",
+            quote + "A" + quote + quote + "B" + quote,
+            quote + "A" + quote + "B" + quote + "C" + quote,
+            "A" + quote + quote + "B"
+        };
+        for (String logOutput : invalidOutputs) {
+            ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-Xlog:logging=trace",
+                                                                      "-Xlog:all=trace:" + logOutput,
+                                                                      "-version");
+            OutputAnalyzer output = new OutputAnalyzer(pb.start());
+            output.shouldHaveExitValue(1);
+            // Ensure error message was logged
+            output.shouldMatch("([Mm]issing terminating quote)"
+                + "|(Could not open log file '')"
+                + "|(Output name can not be partially quoted)");
+        }
+    }
+}
+