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

Bug in w3c/epubcheck/src/main/java/com/adobe/epubcheck/util/PathUtil.java #1181

Closed
kevinhendricks opened this issue Aug 24, 2020 · 1 comment · Fixed by #1206
Closed

Bug in w3c/epubcheck/src/main/java/com/adobe/epubcheck/util/PathUtil.java #1181

kevinhendricks opened this issue Aug 24, 2020 · 1 comment · Fixed by #1206
Assignees
Labels
status: has PR The issue is being processed in a pull request type: bug The issue describes a bug
Milestone

Comments

@kevinhendricks
Copy link

kevinhendricks commented Aug 24, 2020

It is valid to have user.dir value set to root ("/") (and this happens when subprocesses are used to launch the epubcheck via the commandline) but this routine then breaks all paths:

static final String workingDirectory  = System.getProperty("user.dir");
 public static String removeWorkingDirectory(String path)
  {
    if (path == null || path.length() == 0)
    {
      return path;
    }
    return path.replace(workingDirectory, ".");
  }

By replacing all "/" chars in path to "." chars. Given "." chars are valid in filenames and directory names, this replacement is unfixable.

Please make this routine robust to all possible legal values for user.dir

@rdeltour rdeltour self-assigned this Aug 24, 2020
@rdeltour rdeltour added status: accepted Ready to be further processed type: bug The issue describes a bug labels Aug 24, 2020
@rdeltour rdeltour added this to the v4.2.5 milestone Aug 24, 2020
@rdeltour
Copy link
Member

Thanks for the report 👍, I'll look into it tentatively for the next maintenance release!

rdeltour added a commit that referenced this issue Feb 26, 2021
The previous implementation of the `removeWorkingDirectory` utility
method was quite brutal and replaced all occurences of the "user.dir"
system property in paths, even when found in the middle of the path.

The new implementation only replaces the user directory when it occurs
at the beginning of the path, and does nothing when the user directory
is set to the root directory ("/").

Tests included.

Fixes #1181
@rdeltour rdeltour added status: has PR The issue is being processed in a pull request and removed status: accepted Ready to be further processed labels Feb 26, 2021
rdeltour added a commit that referenced this issue Feb 26, 2021
The previous implementation of the `removeWorkingDirectory` utility
method was quite brutal and replaced all occurences of the "user.dir"
system property in paths, even when found in the middle of the path.

The new implementation only replaces the user directory when it occurs
at the beginning of the path, and does nothing when the user directory
is set to the root directory ("/").

Tests included.

Fixes #1181
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: has PR The issue is being processed in a pull request type: bug The issue describes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants