United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7131346 Parsing of boolean arguments to diagnostic commands is broken
JDK-7131346 : Parsing of boolean arguments to diagnostic commands is broken

Details
Type:
Bug
Submit Date:
2012-01-19
Status:
Closed
Updated Date:
2012-03-24
Project Name:
JDK
Resolved Date:
2012-03-24
Component:
hotspot
OS:
generic
Sub-Component:
svc
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
7u4
Fixed Versions:
hs23 (b12)

Related Reports
Backport:
Backport:

Sub Tasks

Description
When executing the diagnostic command GC.heap_dump it is not possible to use the argument "-all=true" to include all objects in heap dump. This usage causes an exception to be thrown from l.70 in diagnosticArgument.cpp:
if (strcasecmp(str, "true") == 0) {
 set_value(true);
} else if (strcasecmp(str, "false") == 0) {
 set_value(false);
} else {
 THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
 "Boolean parsing error in diagnostic command arguments");

                                    

Comments
SUGGESTED FIX

in 
template <> 
void DCmdArgument<bool>::parse_value(const char* str, size_t len, TRAPS)

use the "len" argument to limit strcasecmp to only compare the part of the string that actually contains the "true" or "false" argument:

diff -r 4f25538b54c9 src/share/vm/services/diagnosticArgument.cpp
--- a/src/share/vm/services/diagnosticArgument.cpp
+++ b/src/share/vm/services/diagnosticArgument.cpp
@@ -62,9 +62,9 @@
   if (len == 0) {
     set_value(true);
   } else {
-    if (strcasecmp(str, "true") == 0) {
+    if (strncasecmp(str, "true", len) == 0) {
        set_value(true);
-    } else if (strcasecmp(str, "false") == 0) {
+    } else if (strncasecmp(str, "false", len) == 0) {
        set_value(false);
     } else {
       THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
                                     
2012-01-19
SUGGESTED FIX

The suggested fix would allow invalid boolean values like -all=truefalse.

Here's an improved fix that strictly checks the boolean values:

--- old/src/share/vm/services/diagnosticArgument.cpp  Thu Jan 19 10:36:10 2012
+++ new/src/share/vm/services/diagnosticArgument.cpp  Thu Jan 19 10:36:10 2012
@@ -62,9 +62,9 @@
   if (len == 0) {
     set_value(true);
   } else {
-    if (strcasecmp(str, "true") == 0) {
+    if (len == strlen("true") && strncasecmp(str, "true", len) == 0) {
        set_value(true);
-    } else if (strcasecmp(str, "false") == 0) {
+    } else if (len == strlen("false") && strncasecmp(str, "false", len) == 0) {
        set_value(false);
     } else {
       THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
                                     
2012-01-19
EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/520830f632e7
                                     
2012-01-26
EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/520830f632e7
                                     
2012-01-27
EVALUATION

http://hg.openjdk.java.net/lambda/lambda/hotspot/rev/520830f632e7
                                     
2012-03-22



Hardware and Software, Engineered to Work Together