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

Url repository should respect repo.path for file urls #11687

Merged
merged 1 commit into from Jul 14, 2015

Conversation

@imotov
Copy link
Member

commented Jun 16, 2015

Otherwise its operation will not be allowed by the security manager

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2015

@rmuir can you please take a look at this

@rmuir
rmuir reviewed Jul 2, 2015
View changes
core/src/main/java/org/elasticsearch/common/io/PathUtils.java Outdated
*/
public static Path get(Path[] roots, URI uri) {
return get(roots, Paths.get(uri).normalize().toString());
}

This comment has been minimized.

Copy link
@rmuir

rmuir Jul 2, 2015

Contributor

I think this needs to call PathUtils.get (not Paths.get) so that it does not use the JVM default filesystem, in the case one is set differently in tests.

try {
if ("file".equalsIgnoreCase(url.getProtocol())) {
if (url.getHost() == null || "".equals(url.getHost())) {
// only local file urls are supported

This comment has been minimized.

Copy link
@rmuir

rmuir Jul 2, 2015

Contributor

this looks good. i forgot how crazy URL was here with null versus empty strings for different pieces...

@rmuir

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2015

this looks good to me. i left one minor comment.

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 12, 2015

@imotov let's get this in?

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 13, 2015

We should disable the use of URLs for repositories by default. Later perhaps we can look at adding a white list

@imotov imotov force-pushed the imotov:url-repository-should-use-repo-path branch 2 times, most recently Jul 14, 2015
@imotov

This comment has been minimized.

Copy link
Member Author

commented Jul 14, 2015

@rmuir I added URL whitelist. Could you take another look when you have a chance?

@rmuir

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2015

looks good

Require urls for URL repository to be listed in repositories.url.allowed_urls setting. This change ensures that only authorized URLs can be accessed by elasticsearch
@imotov imotov force-pushed the imotov:url-repository-should-use-repo-path branch to 24a9384 Jul 14, 2015
@imotov imotov merged commit 24a9384 into elastic:master Jul 14, 2015
1 check passed
1 check passed
CLA Commit author is a member of Elasticsearch
Details
@imotov imotov added the >breaking label Jul 14, 2015
@clintongormley clintongormley removed the review label Aug 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.