Skip to content

Commit

Permalink
classpath: URLs are always absolute. Fixes #4188.
Browse files Browse the repository at this point in the history
  • Loading branch information
headius committed Sep 30, 2016
1 parent a4945b6 commit ba74846
Showing 1 changed file with 2 additions and 2 deletions.
Expand Up @@ -194,8 +194,8 @@ private static boolean isAbsolute(String path) {
// uri: are absolute
return true;
}
if (path.startsWith("classpath:/")) {
// classpath URLS are absolute if they start with a slash
if (path.startsWith("classpath:")) {
// classpath URLS are always absolute
return true;
}
return new File(path).isAbsolute();
Expand Down

8 comments on commit ba74846

@kares
Copy link
Member

@kares kares commented on ba74846 Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are they? // cc @mkristian

@headius
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure...but I guess I'm confused how they wouldn't be given that there's no concept of current working directory inside classloader resources.

@mkristian
Copy link
Member

@mkristian mkristian commented on ba74846 Oct 1, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kares
Copy link
Member

@kares kares commented on ba74846 Oct 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought I did see paths such as classpath:some/dir ... I'm not 100% what that means in terms of path relativeness. maybe matching / or a letter followed by ':' (on Windows) is safer.

@mkristian
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive letter on a classpath uri makes no sense as I do not know how to load via a classloader.getResource. getting a drive letter at this point indicates that paths and URIs get mixed up somewhere before - maybe on user's code. rather throw an exception here if you see an drive letter on a classpath uri.

@kares
Copy link
Member

@kares kares commented on ba74846 Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is quite the way it is on windows with require_relative (according to the report) - which this commit was supposed to fix ... dealing with classpath:C:/lib/test

@headius
Copy link
Member Author

@headius headius commented on ba74846 Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The classpath:C:/xxx form is not valid; it's a side effect of require_relative's downstream calls not recognizing classpath: prefix as being in-jar and not subject to filesystem paths. This change actually helps guarantee we won't see invalid URIs like classpath:C:/xxx or classpath:/usr/local/lib/blah.

@kares
Copy link
Member

@kares kares commented on ba74846 Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good, thanks for clarifying!

Please sign in to comment.