7178703: Fix handling of quoted arguments and better error messages in dcmd

Thu, 28 Jun 2012 11:37:28 +0200

author
sla
date
Thu, 28 Jun 2012 11:37:28 +0200
changeset 3905
5a1f452f8f90
parent 3904
ace99a6ffc83
child 3906
04ade88d9712

7178703: Fix handling of quoted arguments and better error messages in dcmd
Reviewed-by: coleenp, mgronlun, rbackman

src/share/vm/prims/whitebox.cpp file | annotate | diff | comparison | revisions
src/share/vm/services/diagnosticCommand.hpp file | annotate | diff | comparison | revisions
src/share/vm/services/diagnosticFramework.cpp file | annotate | diff | comparison | revisions
src/share/vm/services/diagnosticFramework.hpp file | annotate | diff | comparison | revisions
test/serviceability/ParserTest.java file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/prims/whitebox.cpp	Wed Jul 04 15:55:45 2012 -0400
     1.2 +++ b/src/share/vm/prims/whitebox.cpp	Thu Jun 28 11:37:28 2012 +0200
     1.3 @@ -113,6 +113,9 @@
     1.4    int offset = offset_for_field(field_name, object,
     1.5        vmSymbols::string_signature());
     1.6    oop string = object->obj_field(offset);
     1.7 +  if (string == NULL) {
     1.8 +    return NULL;
     1.9 +  }
    1.10    const char* ret = java_lang_String::as_utf8_string(string);
    1.11    return ret;
    1.12  }
     2.1 --- a/src/share/vm/services/diagnosticCommand.hpp	Wed Jul 04 15:55:45 2012 -0400
     2.2 +++ b/src/share/vm/services/diagnosticCommand.hpp	Thu Jun 28 11:37:28 2012 +0200
     2.3 @@ -48,7 +48,7 @@
     2.4             "With no argument this will show a list of available commands. "
     2.5             "'help all' will show help for all commands.";
     2.6    }
     2.7 -  static const char* impact() { return "Low: "; }
     2.8 +  static const char* impact() { return "Low"; }
     2.9    static int num_arguments();
    2.10    virtual void execute(TRAPS);
    2.11  };
    2.12 @@ -60,7 +60,7 @@
    2.13    static const char* description() {
    2.14      return "Print JVM version information.";
    2.15    }
    2.16 -  static const char* impact() { return "Low: "; }
    2.17 +  static const char* impact() { return "Low"; }
    2.18    static int num_arguments() { return 0; }
    2.19    virtual void execute(TRAPS);
    2.20  };
    2.21 @@ -72,7 +72,7 @@
    2.22    static const char* description() {
    2.23      return "Print the command line used to start this VM instance.";
    2.24    }
    2.25 -  static const char* impact() { return "Low: "; }
    2.26 +  static const char* impact() { return "Low"; }
    2.27    static int num_arguments() { return 0; }
    2.28    virtual void execute(TRAPS) {
    2.29      Arguments::print_on(_output);
    2.30 @@ -88,7 +88,7 @@
    2.31        return "Print system properties.";
    2.32      }
    2.33      static const char* impact() {
    2.34 -      return "Low: ";
    2.35 +      return "Low";
    2.36      }
    2.37      static int num_arguments() { return 0; }
    2.38      virtual void execute(TRAPS);
    2.39 @@ -105,7 +105,7 @@
    2.40      return "Print VM flag options and their current values.";
    2.41    }
    2.42    static const char* impact() {
    2.43 -    return "Low: ";
    2.44 +    return "Low";
    2.45    }
    2.46    static int num_arguments();
    2.47    virtual void execute(TRAPS);
    2.48 @@ -121,7 +121,7 @@
    2.49      return "Print VM uptime.";
    2.50    }
    2.51    static const char* impact() {
    2.52 -    return "Low: ";
    2.53 +    return "Low";
    2.54    }
    2.55    static int num_arguments();
    2.56    virtual void execute(TRAPS);
     3.1 --- a/src/share/vm/services/diagnosticFramework.cpp	Wed Jul 04 15:55:45 2012 -0400
     3.2 +++ b/src/share/vm/services/diagnosticFramework.cpp	Thu Jun 28 11:37:28 2012 +0200
     3.3 @@ -75,11 +75,13 @@
     3.4    }
     3.5    // extracting first item, argument or option name
     3.6    _key_addr = &_buffer[_cursor];
     3.7 +  bool arg_had_quotes = false;
     3.8    while (_cursor <= _len - 1 && _buffer[_cursor] != '=' && _buffer[_cursor] != _delim) {
     3.9      // argument can be surrounded by single or double quotes
    3.10      if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') {
    3.11        _key_addr++;
    3.12        char quote = _buffer[_cursor];
    3.13 +      arg_had_quotes = true;
    3.14        while (_cursor < _len - 1) {
    3.15          _cursor++;
    3.16          if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') {
    3.17 @@ -95,16 +97,22 @@
    3.18      _cursor++;
    3.19    }
    3.20    _key_len = &_buffer[_cursor] - _key_addr;
    3.21 +  if (arg_had_quotes) {
    3.22 +    // if the argument was quoted, we need to step past the last quote here
    3.23 +    _cursor++;
    3.24 +  }
    3.25    // check if the argument has the <key>=<value> format
    3.26    if (_cursor <= _len -1 && _buffer[_cursor] == '=') {
    3.27      _cursor++;
    3.28      _value_addr = &_buffer[_cursor];
    3.29 +    bool value_had_quotes = false;
    3.30      // extract the value
    3.31      while (_cursor <= _len - 1 && _buffer[_cursor] != _delim) {
    3.32        // value can be surrounded by simple or double quotes
    3.33        if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') {
    3.34          _value_addr++;
    3.35          char quote = _buffer[_cursor];
    3.36 +        value_had_quotes = true;
    3.37          while (_cursor < _len - 1) {
    3.38            _cursor++;
    3.39            if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') {
    3.40 @@ -120,6 +128,10 @@
    3.41        _cursor++;
    3.42      }
    3.43      _value_len = &_buffer[_cursor] - _value_addr;
    3.44 +    if (value_had_quotes) {
    3.45 +      // if the value was quoted, we need to step past the last quote here
    3.46 +      _cursor++;
    3.47 +    }
    3.48    } else {
    3.49      _value_addr = NULL;
    3.50      _value_len = 0;
    3.51 @@ -185,8 +197,17 @@
    3.52          arg->read_value(iter.key_addr(), iter.key_length(), CHECK);
    3.53          next_argument = next_argument->next();
    3.54        } else {
    3.55 -        THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
    3.56 -          "Unknown argument in diagnostic command");
    3.57 +        const size_t buflen    = 120;
    3.58 +        const size_t argbuflen = 30;
    3.59 +        char buf[buflen];
    3.60 +        char argbuf[argbuflen];
    3.61 +        size_t len = MIN2<size_t>(iter.key_length(), argbuflen - 1);
    3.62 +
    3.63 +        strncpy(argbuf, iter.key_addr(), len);
    3.64 +        argbuf[len] = '\0';
    3.65 +        jio_snprintf(buf, buflen - 1, "Unknown argument '%s' in diagnostic command.", argbuf);
    3.66 +
    3.67 +        THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf);
    3.68        }
    3.69      }
    3.70      cont = iter.next(CHECK);
    3.71 @@ -207,19 +228,21 @@
    3.72  }
    3.73  
    3.74  void DCmdParser::check(TRAPS) {
    3.75 +  const size_t buflen = 256;
    3.76 +  char buf[buflen];
    3.77    GenDCmdArgument* arg = _arguments_list;
    3.78    while (arg != NULL) {
    3.79      if (arg->is_mandatory() && !arg->has_value()) {
    3.80 -      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
    3.81 -              "Missing argument for diagnostic command");
    3.82 +      jio_snprintf(buf, buflen - 1, "The argument '%s' is mandatory.", arg->name());
    3.83 +      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf);
    3.84      }
    3.85      arg = arg->next();
    3.86    }
    3.87    arg = _options;
    3.88    while (arg != NULL) {
    3.89      if (arg->is_mandatory() && !arg->has_value()) {
    3.90 -      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
    3.91 -              "Missing option for diagnostic command");
    3.92 +      jio_snprintf(buf, buflen - 1, "The option '%s' is mandatory.", arg->name());
    3.93 +      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf);
    3.94      }
    3.95      arg = arg->next();
    3.96    }
     4.1 --- a/src/share/vm/services/diagnosticFramework.hpp	Wed Jul 04 15:55:45 2012 -0400
     4.2 +++ b/src/share/vm/services/diagnosticFramework.hpp	Thu Jun 28 11:37:28 2012 +0200
     4.3 @@ -238,6 +238,16 @@
     4.4    static const char* name() { return "No Name";}
     4.5    static const char* description() { return "No Help";}
     4.6    static const char* disabled_message() { return "Diagnostic command currently disabled"; }
     4.7 +  // The impact() method returns a description of the intrusiveness of the diagnostic
     4.8 +  // command on the Java Virtual Machine behavior. The rational for this method is that some
     4.9 +  // diagnostic commands can seriously disrupt the behavior of the Java Virtual Machine
    4.10 +  // (for instance a Thread Dump for an application with several tens of thousands of threads,
    4.11 +  // or a Head Dump with a 40GB+ heap size) and other diagnostic commands have no serious
    4.12 +  // impact on the JVM (for instance, getting the command line arguments or the JVM version).
    4.13 +  // The recommended format for the description is <impact level>: [longer description],
    4.14 +  // where the impact level is selected among this list: {Low, Medium, High}. The optional
    4.15 +  // longer description can provide more specific details like the fact that Thread Dump
    4.16 +  // impact depends on the heap size.
    4.17    static const char* impact() { return "Low: No impact"; }
    4.18    static int num_arguments() { return 0; }
    4.19    outputStream* output() { return _output; }
    4.20 @@ -250,7 +260,7 @@
    4.21      bool has_arg = iter.next(CHECK);
    4.22      if (has_arg) {
    4.23        THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
    4.24 -                "Unknown argument in diagnostic command");
    4.25 +                "The argument list of this diagnostic command should be empty.");
    4.26      }
    4.27    }
    4.28    virtual void execute(TRAPS) { }
     5.1 --- a/test/serviceability/ParserTest.java	Wed Jul 04 15:55:45 2012 -0400
     5.2 +++ b/test/serviceability/ParserTest.java	Thu Jun 28 11:37:28 2012 +0200
     5.3 @@ -20,6 +20,7 @@
     5.4          testNanoTime();
     5.5          testJLong();
     5.6          testBool();
     5.7 +        testQuotes();
     5.8          testMemorySize();
     5.9      }
    5.10  
    5.11 @@ -95,6 +96,33 @@
    5.12          parse(name, "false", "", args);
    5.13      }
    5.14  
    5.15 +    public void testQuotes() throws Exception {
    5.16 +        String name = "name";
    5.17 +        DiagnosticCommand arg1 = new DiagnosticCommand(name,
    5.18 +                "desc", DiagnosticArgumentType.STRING,
    5.19 +                false, null);
    5.20 +        DiagnosticCommand arg2 = new DiagnosticCommand("arg",
    5.21 +                "desc", DiagnosticArgumentType.STRING,
    5.22 +                false, null);
    5.23 +        DiagnosticCommand[] args = {arg1, arg2};
    5.24 +
    5.25 +        // try with a quoted value
    5.26 +        parse(name, "Recording 1", name + "=\"Recording 1\"", args);
    5.27 +        // try with a quoted argument
    5.28 +        parse(name, "myrec", "\"" + name + "\"" + "=myrec", args);
    5.29 +        // try with both a quoted value and a quoted argument
    5.30 +        parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\"", args);
    5.31 +
    5.32 +        // now the same thing but with other arguments after
    5.33 +
    5.34 +        // try with a quoted value
    5.35 +        parse(name, "Recording 1", name + "=\"Recording 1\",arg=value", args);
    5.36 +        // try with a quoted argument
    5.37 +        parse(name, "myrec", "\"" + name + "\"" + "=myrec,arg=value", args);
    5.38 +        // try with both a quoted value and a quoted argument
    5.39 +        parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\",arg=value", args);
    5.40 +    }
    5.41 +
    5.42      public void testMemorySize() throws Exception {
    5.43          String name = "name";
    5.44          String defaultValue = "1024";

mercurial