Fix handling of spaces in Windows paths

[jasontedor]

This commit fixes the handling of spaces in Windows paths. The current
mechanism works fine in a path that contains a single space, but fails
on a path that contains multiple spaces. With this commit, that is no
longer the case.

Relates #20809, relates #21525

[jasontedor]

I've manually tested this for both running Elasticsearch from the command line and the service, and with paths that contain zero, one, two, and three spaces and everything works as expected.

[ngbrown]

When setting environment variables, the full path should be included in the quotes.

[ngbrown]

The quotes should surround the entire path, including the .exe.

[ngbrown]

The quotes should surround the entire path, including the .exe.

[jasontedor]

Why? I tested it with this change. Do you have an example where this would break without your suggestion?

[rjernst]

This LGTM. Should we consider (in a follow up) having our integ test dir which contains a space have two spaces (I did not realize it was possible to have different behavior for one vs two spaces).

[jasontedor]

Should we consider (in a follow up) having our integ test dir which contains a space have two spaces (I did not realize it was possible to have different behavior for one vs two spaces)

Yes, I was thinking the same thing. Lacking packaging tests, that's test best that we can do but it's been really effective for us in the past.

[jasontedor]

Thanks @rjernst.

[yannlebel]

When I read this it seems the issue is not closed and included in a release package but when I download the elasticsearch zip from here https://www.elastic.co/downloads/elasticsearch it's already included. Did something went wrong?

This fix in not working for me. I tested the version 5.1.1 on windows 7 (x64) and windows server 2012 R2 (x64) standard and had the same issue every time.

The quote around the %JAVA_HOME% and %JAVA% variables are not placed correctly which prevent the scripts to start the node and to detect the JVM version which always default the service installation to the x86 version.

My JAVA_HOME variable is set with the value: C:\Program Files\Java\jdk1.8.0_111
My installation folder for elasticsearch is: D:\ELK\elasticsearch\

######Result of the elasticsearch.bat execution before the fix:

######Result of the elasticsearch-service.bat execution before the fix:

To fix the problem I had to do the following changes:

• In elastisearch.bat
  • Line 82
    • From: %JAVA% %ES_JAVA_OPTS% %ES_PARAMS% -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams!
    • To: "%JAVA%" %ES_JAVA_OPTS% %ES_PARAMS% -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams!
• In elasticsearch.in.bat
  • Line 4
    • From: set JAVA="%JAVA_HOME%"\bin\java.exe
    • To: set JAVA=%JAVA_HOME%\bin\java.exe
  • Line 8
    • From: IF NOT EXIST %JAVA% (
    • To: IF NOT EXIST "%JAVA%" (
• In Elasticsearch-service.bat
  • Line 32
    • From: SET JAVA="%JAVA_HOME%"\bin\java.exe
    • To: SET JAVA=%JAVA_HOME%\bin\java.exe
  • Line 36
    • From: IF NOT EXIST %JAVA% (
    • To: IF NOT EXIST "%JAVA%" (
  • Line 55
    • From: %JAVA% -Xmx50M -version > nul 2>&1
    • To: "%JAVA%" -Xmx50M -version > nul 2>&1
  • Line 62
    • From: %JAVA% -Xmx50M -version 2>&1 | "%windir%\System32\find" "64-Bit" >nul:
    • To: "%JAVA%" -Xmx50M -version 2>&1 | "%windir%\System32\find" "64-Bit" >nul:
  • Line 149
    • From: if exist "%JAVA_HOME%"\jre\bin\server\jvm.dll (
    • To: if exist "%JAVA_HOME%\jre\bin\server\jvm.dll" (
  • Line 155
    • From: if exist "%JAVA_HOME%"\bin\server\jvm.dll (
    • To: if exist "%JAVA_HOME%\bin\server\jvm.dll" (
  • Line 161
    • From: if exist "%JAVA_HOME%"\bin\client\jvm.dll (
    • To: if exist "%JAVA_HOME%\bin\client\jvm.dll" (

Here are the files with the fixes:

elasticsearch-service.bat.txt
elasticsearch.bat.txt
elasticsearch.in.bat.txt

Hope it helps.

[jasontedor]

@yannlebel If you remove the trailing backslash from JAVA_HOME, what is packaged today works fine. There is an open PR (#22132) to address this.

[yannlebel]

Sorry my mistake I didn't read the changes in the commit correctly and thought this was the PR and #22132 was an issue. I tried without the trailing slash and indead it is working.