8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently

[yano-masanori at fujitsu.com]

Hello.

I would like to contribute for JDK-8250678.

The 'parse' method of ModuleDescriptor.Version class behaves incorrectly 
when the input string contains consecutive delimiters.

The 'parse' method treats strings as three sections, but the parsing method 
differs between the method for the version sections and the ones for others.
In version sections, the 'parse' method takes a single character in a loop and 
determines whether it is a delimiter. In pre and build sections, the parse method 
takes two characters in a loop and determines whether the second character is the delimiter.
Therefore, if the string contains a sequence of delimiters in pre or build section, 
the 'parse' method treats character at the odd-numbered position as a token and 
the one at even-numbered as a delimiter

A string containing consecutive delimiters is an incorrect version string, 
but this behavior does not follow the API specification.
https://download.java.net/java/early_access/jdk16/docs/api/java.base/java/lang/module/ModuleDescriptor.Version.html

Therefore, I propose to fix the parsing method of the pre section and 
the build section in the same way as the version.

Please sponsor the following change.

diff -r bdc20ee1a68d src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
--- a/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java	Fri Sep 04 23:51:26 2020 -0400
+++ b/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java	Wed Oct 28 17:06:47 2020 +0900
@@ -1053,13 +1053,6 @@
 
             while (i < n) {
                 c = v.charAt(i);
-                if (c >= '0' && c <= '9')
-                    i = takeNumber(v, i, pre);
-                else
-                    i = takeString(v, i, pre);
-                if (i >= n)
-                    break;
-                c = v.charAt(i);
                 if (c == '.' || c == '-') {
                     i++;
                     continue;
@@ -1068,6 +1061,10 @@
                     i++;
                     break;
                 }
+                if (c >= '0' && c <= '9')
+                    i = takeNumber(v, i, pre);
+                else
+                    i = takeString(v, i, pre);
             }
 
             if (c == '+' && i >= n)
@@ -1075,17 +1072,14 @@
 
             while (i < n) {
                 c = v.charAt(i);
+                if (c == '.' || c == '-' || c == '+') {
+                    i++;
+                    continue;
+                }
                 if (c >= '0' && c <= '9')
                     i = takeNumber(v, i, build);
                 else
                     i = takeString(v, i, build);
-                if (i >= n)
-                    break;
-                c = v.charAt(i);
-                if (c == '.' || c == '-' || c == '+') {
-                    i++;
-                    continue;
-                }
             }
 
             this.version = v;
diff -r bdc20ee1a68d test/jdk/java/lang/module/VersionTest.java
--- a/test/jdk/java/lang/module/VersionTest.java	Fri Sep 04 23:51:26 2020 -0400
+++ b/test/jdk/java/lang/module/VersionTest.java	Wed Oct 28 17:06:47 2020 +0900
@@ -148,6 +148,8 @@
             { "1",             "1.0.0" },
             { "1.0",           "1.0.0" },
             { "1.0-beta",      "1.0.0-beta" },
+            { "1.0-1.1",       "1.0-1..1" },
+            { "1.0-1+1",       "1.0-1.+1" },
 
         };
     }

Regards,
Masanori Yano