# HG changeset patch # User mlarsson # Date 1472195962 -7200 # Fri Aug 26 09:19:22 2016 +0200 # Node ID 4c2362f746cc2e9f8361dc5a0ae60d42c9cdb106 # Parent 8c8e9b96bd7c43888285a5936706788a1820b985 imported patch 8157948 diff --git a/src/share/vm/logging/log.cpp b/src/share/vm/logging/log.cpp --- a/src/share/vm/logging/log.cpp +++ b/src/share/vm/logging/log.cpp @@ -1161,7 +1161,7 @@ // Attempt to log to a directory (existing log not a regular file) create_directory(target_name); - LogFileOutput bad_file("tmplogdir"); + LogFileOutput bad_file("file=tmplogdir"); assert(bad_file.initialize("", &ss) == false, "file was initialized " "when there was an existing directory with the same name"); assert(strstr(ss.as_string(), "tmplogdir is not a regular file") != NULL, diff --git a/src/share/vm/logging/logConfiguration.cpp b/src/share/vm/logging/logConfiguration.cpp --- a/src/share/vm/logging/logConfiguration.cpp +++ b/src/share/vm/logging/logConfiguration.cpp @@ -107,6 +107,54 @@ FREE_C_HEAP_ARRAY(LogOutput*, _outputs); } +// Normalizes the given LogOutput name to type=name form. +static bool normalize_output_name(const char* full_name, char* buffer, size_t len, outputStream* errstream) { + const char* start_quote = strchr(full_name, '"'); + const char* equals = strchr(full_name, '='); + const bool quoted = start_quote != NULL; + const bool is_stdout_or_stderr = (strcmp(full_name, "stdout") == 0 || strcmp(full_name, "stderr") == 0); + + // ignore equals sign within quotes + if (quoted && equals > start_quote) { + equals = NULL; + } + + const char* prefix = ""; + size_t prefix_len = 0; + const char* name = full_name; + if (equals != NULL) { + // split on equals sign + name = equals + 1; + prefix = full_name; + prefix_len = equals - full_name + 1; + } else if (!is_stdout_or_stderr) { + prefix = "file="; + prefix_len = strlen(prefix); + } + size_t name_len = strlen(name); + + if (quoted) { + const char* end_quote = strchr(start_quote + 1, '"'); + if (end_quote == NULL) { + errstream->print_cr("Output name has opening quote but is missing a terminating quote."); + return false; + } + if (start_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 false; + } + // strip start and end quote + name++; + name_len -= 2; + } + + int ret = jio_snprintf(buffer, len, "%.*s%.*s", prefix_len, prefix, name_len, name); + assert(ret > 0, "buffer issue"); + return true; +} + size_t LogConfiguration::find_output(const char* name) { for (size_t i = 0; i < _n_outputs; i++) { if (strcmp(_outputs[i]->name(), name) == 0) { @@ -116,39 +164,14 @@ return SIZE_MAX; } -LogOutput* LogConfiguration::new_output(char* name, const char* options, outputStream* errstream) { - const char* type; - char* equals_pos = strchr(name, '='); - if (equals_pos == NULL) { - type = "file"; - } else { - *equals_pos = '\0'; - type = name; - 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* LogConfiguration::new_output(const char* name, + const char* options, + outputStream* errstream) { LogOutput* output; - if (strcmp(type, "file") == 0) { + if (strncmp(name, "file=", strlen("file=")) == 0) { output = new LogFileOutput(name); } else { - errstream->print_cr("Unsupported log output type."); + errstream->print_cr("Unsupported log output type: %s", name); return NULL; } @@ -364,25 +387,35 @@ ConfigurationLock cl; size_t idx; - if (outputstr[0] == '#') { - int ret = sscanf(outputstr+1, SIZE_FORMAT, &idx); + if (outputstr[0] == '#') { // Output specified using index + int ret = sscanf(outputstr + 1, SIZE_FORMAT, &idx); if (ret != 1 || idx >= _n_outputs) { errstream->print_cr("Invalid output index '%s'", outputstr); return false; } - } else { - idx = find_output(outputstr); + } else { // Output specified using name + // Normalize the name, stripping quotes and ensures it includes type prefix + size_t len = strlen(outputstr) + strlen("file=") + 1; + char* normalized = NEW_C_HEAP_ARRAY(char, len, mtLogging); + if (!normalize_output_name(outputstr, normalized, len, errstream)) { + return false; + } + + idx = find_output(normalized); if (idx == SIZE_MAX) { - char* tmp = os::strdup_check_oom(outputstr, mtLogging); - LogOutput* output = new_output(tmp, output_options, errstream); - os::free(tmp); - if (output == NULL) { - return false; + // Attempt to create and add the output + LogOutput* output = new_output(normalized, output_options, errstream); + if (output != NULL) { + idx = add_output(output); } - idx = add_output(output); } else if (output_options != NULL && strlen(output_options) > 0) { errstream->print_cr("Output options for existing outputs are ignored."); } + + FREE_C_HEAP_ARRAY(char, normalized); + if (idx == SIZE_MAX) { + return false; + } } configure_output(idx, expr, decorators); notify_update_listeners(); diff --git a/src/share/vm/logging/logConfiguration.hpp b/src/share/vm/logging/logConfiguration.hpp --- a/src/share/vm/logging/logConfiguration.hpp +++ b/src/share/vm/logging/logConfiguration.hpp @@ -59,7 +59,7 @@ static size_t _n_listener_callbacks; // Create a new output. Returns NULL if failed. - static LogOutput* new_output(char* name, const char* options, outputStream* errstream); + static LogOutput* new_output(const 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); diff --git a/src/share/vm/logging/logFileOutput.cpp b/src/share/vm/logging/logFileOutput.cpp --- a/src/share/vm/logging/logFileOutput.cpp +++ b/src/share/vm/logging/logFileOutput.cpp @@ -45,7 +45,8 @@ _file_name(NULL), _archive_name(NULL), _archive_name_len(0), _rotate_size(DefaultFileSize), _file_count(DefaultFileCount), _current_size(0), _current_file(0), _rotation_semaphore(1) { - _file_name = make_file_name(name, _pid_str, _vm_start_time_str); + assert(strstr(name, "file=") == name, "invalid output name '%s': missing prefix", name); + _file_name = make_file_name(name + strlen("file="), _pid_str, _vm_start_time_str); } void LogFileOutput::set_file_name_parameters(jlong vm_start_time) { diff --git a/test/native/logging/test_logConfiguration.cpp b/test/native/logging/test_logConfiguration.cpp --- a/test/native/logging/test_logConfiguration.cpp +++ b/test/native/logging/test_logConfiguration.cpp @@ -289,3 +289,30 @@ } EXPECT_STREQ("", ss.as_string()) << "Error reported while parsing: " << ss.as_string(); } + +TEST_F(LogConfigurationTest, output_name_normalization) { + const char* patterns[] = { "%s", "file=%s", "\"%s\"", "file=\"%s\"" }; + char buf[1 * K]; + for (size_t i = 0; i < ARRAY_SIZE(patterns); i++) { + int ret = jio_snprintf(buf, sizeof(buf), patterns[i], TestLogFileName); + ASSERT_NE(-1, ret); + set_log_config(buf, "logging=trace"); + EXPECT_TRUE(is_described("#2: ")); + EXPECT_TRUE(is_described(TestLogFileName)); + EXPECT_FALSE(is_described("#3: ")) + << "duplicate file output due to incorrect normalization for pattern: " << patterns[i]; + } + + // Make sure prefixes are ignored when used within quotes + // (this should create a log with "file=" in its filename) + int ret = jio_snprintf(buf, sizeof(buf), "\"file=%s\"", TestLogFileName); + ASSERT_NE(-1, ret); + set_log_config(buf, "logging=trace"); + EXPECT_TRUE(is_described("#3: ")) << "prefix within quotes not ignored as it should be"; + set_log_config(buf, "all=off"); + + // Remove the extra log file created + ret = jio_snprintf(buf, sizeof(buf), "file=%s", TestLogFileName); + ASSERT_NE(-1, ret); + delete_file(buf); +} diff --git a/test/native/logging/test_logFileOutput.cpp b/test/native/logging/test_logFileOutput.cpp --- a/test/native/logging/test_logFileOutput.cpp +++ b/test/native/logging/test_logFileOutput.cpp @@ -29,7 +29,7 @@ #include "utilities/globalDefinitions.hpp" #include "utilities/ostream.hpp" -static const char* name = "testlog.pid%p.%t.log"; +static const char* name = "file=testlog.pid%p.%t.log"; // Test parsing a bunch of valid file output options TEST(LogFileOutput, parse_valid) { # HG changeset patch # User mlarsson # Date 1472211703 -7200 # Fri Aug 26 13:41:43 2016 +0200 # Node ID 19c963a296169fde8b77316c7f52b6368dbbd6d2 # Parent 4c2362f746cc2e9f8361dc5a0ae60d42c9cdb106 [mq]: 8157948.01 diff --git a/src/share/vm/logging/logConfiguration.cpp b/src/share/vm/logging/logConfiguration.cpp --- a/src/share/vm/logging/logConfiguration.cpp +++ b/src/share/vm/logging/logConfiguration.cpp @@ -44,6 +44,9 @@ LogConfiguration::UpdateListenerFunction* LogConfiguration::_listener_callbacks = NULL; size_t LogConfiguration::_n_listener_callbacks = 0; +// LogFileOutput is the default type of output, its type prefix should be used if no type was specified +static const char* implicit_output_prefix = LogFileOutput::Prefix; + // Stack object to take the lock for configuring the logging. // Should only be held during the critical parts of the configuration // (when calling configure_output or reading/modifying the outputs array). @@ -108,6 +111,7 @@ } // Normalizes the given LogOutput name to type=name form. +// For example, foo, "foo", file="foo", will all be normalized to file=foo (no quotes, prefixed). static bool normalize_output_name(const char* full_name, char* buffer, size_t len, outputStream* errstream) { const char* start_quote = strchr(full_name, '"'); const char* equals = strchr(full_name, '='); @@ -128,7 +132,7 @@ prefix = full_name; prefix_len = equals - full_name + 1; } else if (!is_stdout_or_stderr) { - prefix = "file="; + prefix = implicit_output_prefix; prefix_len = strlen(prefix); } size_t name_len = strlen(name); @@ -168,7 +172,7 @@ const char* options, outputStream* errstream) { LogOutput* output; - if (strncmp(name, "file=", strlen("file=")) == 0) { + if (strncmp(name, LogFileOutput::Prefix, strlen(LogFileOutput::Prefix)) == 0) { output = new LogFileOutput(name); } else { errstream->print_cr("Unsupported log output type: %s", name); @@ -395,7 +399,7 @@ } } else { // Output specified using name // Normalize the name, stripping quotes and ensures it includes type prefix - size_t len = strlen(outputstr) + strlen("file=") + 1; + size_t len = strlen(outputstr) + strlen(implicit_output_prefix) + 1; char* normalized = NEW_C_HEAP_ARRAY(char, len, mtLogging); if (!normalize_output_name(outputstr, normalized, len, errstream)) { return false; diff --git a/src/share/vm/logging/logFileOutput.cpp b/src/share/vm/logging/logFileOutput.cpp --- a/src/share/vm/logging/logFileOutput.cpp +++ b/src/share/vm/logging/logFileOutput.cpp @@ -31,6 +31,7 @@ #include "utilities/globalDefinitions.hpp" #include "utilities/defaultStream.hpp" +const char* LogFileOutput::Prefix = "file="; const char* LogFileOutput::FileOpenMode = "a"; const char* LogFileOutput::PidFilenamePlaceholder = "%p"; const char* LogFileOutput::TimestampFilenamePlaceholder = "%t"; @@ -45,8 +46,8 @@ _file_name(NULL), _archive_name(NULL), _archive_name_len(0), _rotate_size(DefaultFileSize), _file_count(DefaultFileCount), _current_size(0), _current_file(0), _rotation_semaphore(1) { - assert(strstr(name, "file=") == name, "invalid output name '%s': missing prefix", name); - _file_name = make_file_name(name + strlen("file="), _pid_str, _vm_start_time_str); + assert(strstr(name, Prefix) == name, "invalid output name '%s': missing prefix: %s", name, Prefix); + _file_name = make_file_name(name + strlen(Prefix), _pid_str, _vm_start_time_str); } void LogFileOutput::set_file_name_parameters(jlong vm_start_time) { diff --git a/src/share/vm/logging/logFileOutput.hpp b/src/share/vm/logging/logFileOutput.hpp --- a/src/share/vm/logging/logFileOutput.hpp +++ b/src/share/vm/logging/logFileOutput.hpp @@ -91,6 +91,7 @@ return _name; } + static const char* Prefix; static void set_file_name_parameters(jlong start_time); };