Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if census endpoint is a URL before trying to parse it as a path. #10

Closed
wants to merge 2 commits into from

Conversation

@JornVernee
Copy link
Member

JornVernee commented Jun 27, 2019

When running git jcheck --local I'm getting the following exception:

Exception in thread "main" java.nio.file.InvalidPathException: Illegal char <:> at index 5: https://openjdk.java.net/census.xml
        at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
        at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
        at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
        at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
        at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:229)
        at java.base/java.nio.file.Path.of(Path.java:147)
        at org.openjdk.skara.cli/org.openjdk.skara.cli.GitJCheck.main(GitJCheck.java:159)
        at org.openjdk.skara.cli/org.openjdk.skara.cli.GitSkara.main(GitSkara.java:130)

Looking into the source code this seems to be because the fallback endpoint for getting the OpenJDK census is used, a URL, which is not a valid path (at least not on Windows), so the check to see if we're dealing with a path or not fails when parsing the path.

This PR switches to a regular expression for checking whether the endpoint is a URL or not (by checking if it starts with http(s)://).

Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Approvers

  • Erik Helin (ehelin - Reviewer)
@bridgekeeper
Copy link

bridgekeeper bot commented Jun 27, 2019

👋 Welcome back jvernee! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added cli rfr labels Jun 27, 2019
@mlbridge
Copy link

mlbridge bot commented Jun 27, 2019

Webrevs

Copy link
Member

edvbld left a comment

Thanks @JornVernee for fixing this! As you probably have guessed, neither me nor @rwestberg use Windows on a daily basis (although we test on it from time to time). Just two minor comments inline.

@openjdk
Copy link

openjdk bot commented Jun 27, 2019

This PR has been reviewed by Jorn Vernee (jvernee - no project role) - comment added.

@openjdk
Copy link

openjdk bot commented Jun 27, 2019

This PR has been reviewed by Erik Helin (ehelin - Reviewer) - comment added.

@edvbld
edvbld approved these changes Jun 28, 2019
Copy link
Member

edvbld left a comment

Looks good now, thanks!

@openjdk openjdk bot removed the rfr label Jun 28, 2019
@openjdk
Copy link

openjdk bot commented Jun 28, 2019

The PR review by Erik Helin (ehelin - Reviewer) has been updated - changes are approved.

@openjdk
Copy link

openjdk bot commented Jun 28, 2019

@JornVernee This change can now be integrated. The commit message will be:

Check if census endpoint is a URL before trying to parse it as a path.

Reviewed-by: ehelin
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@edvbld) but any other Committer may sponsor as well.

  • To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@openjdk openjdk bot added the ready label Jun 28, 2019
@edvbld
Copy link
Member

edvbld commented Jun 28, 2019

@JornVernee I just nominated you to become a commiter in Skara, see https://mail.openjdk.java.net/pipermail/skara-dev/2019-June/000109.html for details.

@JornVernee
Copy link
Member Author

JornVernee commented Jun 28, 2019

/integrate

@openjdk
Copy link

openjdk bot commented Jun 28, 2019

@JornVernee
Your change (at version b09c2d0) is now ready to be sponsored by a Committer.

@openjdk openjdk bot added the sponsor label Jun 28, 2019
@JornVernee
Copy link
Member Author

JornVernee commented Jun 28, 2019

@edvbld Thanks for the nomination :)

@edvbld
Copy link
Member

edvbld commented Jun 28, 2019

/sponsor

@openjdk
Copy link

openjdk bot commented Jun 28, 2019

@edvbld @JornVernee Pushed as commit d29c19e.

@openjdk openjdk bot closed this Jun 28, 2019
@JornVernee JornVernee deleted the JornVernee:path_url branch Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.