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

Escape CLI parameters when starting elasticsearch #12709

Closed
wants to merge 1 commit into from

Conversation

xuzha
Copy link
Contributor

@xuzha xuzha commented Aug 6, 2015

I'm not sure if I should do this.

Closes #12677

@dakrone
Copy link
Member

dakrone commented Aug 7, 2015

I actually think maybe we should switch to \"$@\" instead of \"$*\", see http://www.bashguru.com/2009/11/how-to-pass-arguments-to-shell-script.html for the difference when quoted (particularly for files with spaces in the name).

@xuzha
Copy link
Contributor Author

xuzha commented Aug 7, 2015

Hmm, probably I am not quite understand. But I don't think there is any difference between \"$*\" and \"$@\", we escape the double quotes there which will be passed into java program, and $* and $@ became same.

Try this dirty bash script

#!/bin/bash

function print_args_at {
    printf "%s\n" "$@"
}

function print_args_star {
    printf "%s\n" "$*"
}

function print_args_star1 {
    printf "%s\n" \"$*\"
}

function print_args_star2 {
    printf "%s\n" \"$@\"
}

print_args_at "one" "two three" "four"
print_args_star "one" "two three" "four"
print_args_star1 "one" "two three" "four"
print_args_star2 "one" "two three" "four"

@rmuir
Copy link
Contributor

rmuir commented Aug 7, 2015

I am working to make integration tests always use spaces in directory names (some fixes are needed) to blast out these bugs. But I agree with @dakrone here.

@xuzha
Copy link
Contributor Author

xuzha commented Aug 7, 2015

OK, thanks guys, updated.

@clintongormley clintongormley changed the title Escape the parameters. Escape CLI parameters when starting elasticsearch Aug 7, 2015
@clintongormley clintongormley added >bug review :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Aug 7, 2015
@clintongormley
Copy link

@spinscale @xuzha is this PR related to #12709 ?

@spinscale
Copy link
Contributor

Does this work for you? ./bin/elasticsearch -Des.pidfile="/path/with space/es.pid" - results in this error to me:

Exception in thread "main" java.nio.file.AccessDeniedException: /path
    at sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
    at sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:384)
    at java.nio.file.Files.createDirectory(Files.java:674)
    at java.nio.file.Files.createAndCheckIsDirectory(Files.jav
...

Also, calling something like /bin/elasticsearch --http.cors.enabled true --http.cors.allow-origin 'foo' results in this as only a single argument is passed?

ERROR: Parameter [http.cors.enabled true http.cors.allow-origin foo] needs value

@xuzha
Copy link
Contributor Author

xuzha commented Aug 7, 2015

@spinscale You are right, adding " " make them only a single argument is passed. But AccessDeniedException is the expected exception, right?

Anyway, Robert has a much better fix 12710. Ill just close this dumb one.

@xuzha xuzha closed this Aug 7, 2015
@xuzha xuzha deleted the xu-escape branch August 7, 2015 16:27
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

elasticsearch script does not work with arguments that have spaces
6 participants