RFR: 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently
Could you please review the 8250678 bug fixes? 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/jdk18/docs/api/java.base/java/la... Therefore, I propose to fix the parsing method of pre and build section in the same way as the version. ------------- Commit messages: - 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently Changes: https://git.openjdk.java.net/jdk/pull/5679/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5679&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8250678 Stats: 26 lines in 2 files changed: 11 ins; 14 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5679.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5679/head:pull/5679 PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
I thin this is okay, just look unusual to have repeated punctuation creating the case where a component is empty. @mbreinhold may wish to comment on this PR. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Mon, 27 Sep 2021 08:22:02 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
I think this is okay, just maybe unusual to have repeated punctuation creating the case where a component is empty. @mbreinhold may wish to comment on this PR.
@AlanBateman I can't seem to get a comment on this PR from @mbreinhold , so could you please review it? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Mon, 27 Sep 2021 08:22:02 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
I think this is okay, just maybe unusual to have repeated punctuation creating the case where a component is empty. @mbreinhold may wish to comment on this PR.
@AlanBateman I have been waiting for a reply from @mbreinhold , but I haven't received it yet. I would like to contribute this PR as soon as possible. Do you have any ideas on how to do it? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
@mbreinhold Could you comment on this pull request? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Thu, 30 Sep 2021 11:26:12 GMT, Masanori Yano <myano@openjdk.org> wrote:
@mbreinhold Could you comment on this pull request?
This is somewhat tricky code. I’ll take a look, but give me a few days. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Wed, 6 Oct 2021 18:40:36 GMT, Mark Reinhold <mr@openjdk.org> wrote:
@mbreinhold Could you comment on this pull request?
@mbreinhold Could you comment on this pull request?
This is somewhat tricky code. I’ll take a look, but give me a few days.
@mbreinhold I waited for about a week, but I haven't received an answer yet. Could you comment on this? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Wed, 6 Oct 2021 18:40:36 GMT, Mark Reinhold <mr@openjdk.org> wrote:
@mbreinhold Could you comment on this pull request?
@mbreinhold Could you comment on this pull request?
This is somewhat tricky code. I’ll take a look, but give me a few days.
@mbreinhold (@AlanBateman ) Could you tell me how many more days do you need for your confirmation? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Wed, 6 Oct 2021 18:40:36 GMT, Mark Reinhold <mr@openjdk.org> wrote:
@mbreinhold Could you comment on this pull request?
@mbreinhold Could you comment on this pull request?
This is somewhat tricky code. I’ll take a look, but give me a few days.
@mbreinhold @AlanBateman Could you please reply to the above comments? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Wed, 6 Oct 2021 18:40:36 GMT, Mark Reinhold <mr@openjdk.org> wrote:
@mbreinhold Could you comment on this pull request?
@mbreinhold Could you comment on this pull request?
This is somewhat tricky code. I’ll take a look, but give me a few days.
@mbreinhold @AlanBateman Are you aware of my comment? Could you please reply? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
I reviewed the change. It is reasonable to fix the parsing of the pre-release version and the build version be consistent with parsing of the version number which currently skips consecutive delimiters. The spec is unclear on whether an empty token separated by the delimiters is ignored. The spec may needs some clarification. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Tue, 2 Nov 2021 20:09:21 GMT, Mandy Chung <mchung@openjdk.org> wrote:
I reviewed the change. It is reasonable to fix the parsing of the pre-release version and the build version be consistent with parsing of the version number which currently skips consecutive delimiters.
The spec is unclear on whether an empty token separated by the delimiters is ignored. The spec may needs some clarification.
Yes, I think we should fix the consistency issues although the cases where the inconsistency show up seem unusual. In two minds on whether to should do the spec clarification as part of this PR or create a separate issue. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
Thank you for reviewing. Does clarifying the spec in this PR mean modifying the comments in javadoc? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
I would suggest to create a separate issue to follow up the spec clarification and keep this PR to fix the implementation. The version parsing code is tricky. The fix is straight-forward, just moving the check of the delimiters as the first check when iterating the char sequence. I think it's fine for this fix to go in first. @AlanBateman what do you think? ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 5 Nov 2021 16:51:02 GMT, Mandy Chung <mchung@openjdk.org> wrote:
I would suggest to create a separate issue to follow up the spec clarification and keep this PR to fix the implementation.
The version parsing code is tricky. The fix is straight-forward, just moving the check of the delimiters as the first check when iterating the char sequence. I think it's fine for this fix to go in first. @AlanBateman what do you think?
I agree, let's separate the spec clarification to a separate issue and fix the consistency issue with this PR. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
Marked as reviewed by mchung (Reviewer). I created https://bugs.openjdk.java.net/browse/JDK-8276826 to follow up the spec clarification. ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
Marked as reviewed by alanb (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
On Fri, 24 Sep 2021 11:28:09 GMT, Masanori Yano <myano@openjdk.org> wrote:
Could you please review the 8250678 bug fixes?
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/jdk18/docs/api/java.base/java/la...
Therefore, I propose to fix the parsing method of pre and build section in the same way as the version.
This pull request has now been integrated. Changeset: e1985947 Author: Masanori Yano <myano@openjdk.org> Committer: Alan Bateman <alanb@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/e198594753b0b0653256923586c7f4ec9e3c... Stats: 26 lines in 2 files changed: 11 ins; 14 del; 1 mod 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently Reviewed-by: mchung, alanb ------------- PR: https://git.openjdk.java.net/jdk/pull/5679
participants (4)
-
Alan Bateman
-
Mandy Chung
-
Mark Reinhold
-
Masanori Yano