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

Ban PathUtils.get (for now, until we fix the two remaining issues) #11069

Merged
merged 2 commits into from May 11, 2015

Conversation

@rmuir
Copy link
Contributor

commented May 8, 2015

The idea is to ban what will only cause securityexceptions. Instead paths should really be resolved against the ones configured in Environment.

Once we fix #11065 and #11068, then maybe we can make the whole class private/package-private in .env package somewhere.

I fixed the remaining "bogus" use cases of the method in this PR so we are only left with the real issues.

@rmuir rmuir added the v2.0.0-beta1 label May 8, 2015
@rmuir

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2015

I moved tmpdir stuff to Environment.tmpFile() as well. IMO, Files.createTempFile()/Directory() that do not take Path should be banned as well as a followup PR.

Two reasons for that:

  1. it gets initialized in environment once, we ensure it is created and give permissions to it, and is not mutable (e.g. impacted by sysprop changes or whatever).
  2. its consistent with just accessing all paths from environment.
@rjernst

This comment has been minimized.

Copy link
Member

commented May 8, 2015

LGTM

rmuir added a commit that referenced this pull request May 11, 2015
Ban PathUtils.get (for now, until we fix the two remaining issues)
@rmuir rmuir merged commit 4b345ca into elastic:master May 11, 2015
1 check passed
1 check passed
CLA Commit author is a member of Elasticsearch
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.