Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 323054

Summary:

Path elements passed to Rhino need to be escaped properly

Product:

[WebTools] JSDT

Reporter:

Michael Rennie <Michael_Rennie>

Component:

Debug

Assignee:

Michael Rennie <Michael_Rennie>

Status:

RESOLVED FIXED

QA Contact:

Simon Kaegi <simon_kaegi>

Severity:

blocker

Priority:

CC:

thatnitind

Version:

3.2

Target Milestone:

3.3 M2

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
proper escaping	none
possible fix	none

Description Michael Rennie

2010-08-18 11:18:13 EDT

code from HEAD, FUP to bug 289107.

Smoke testing on Windows and Mac I found that the load(...) section of the command line we use to launch Rhino was blowing up. The problem was that we were not escaping windows path elements properly.

Comment 1 Michael Rennie

2010-08-18 12:05:43 EDT

Created attachment 176914 [details]
proper escaping

The patch properly escapes paths. But for some reason the program args '-e load(...)' when passed to Rhino do not actually load any scripts on Windows (but works fine on Linux and Mac). If I copy the load(..) command from the process properties and run if from DebugShell it works fine....

Comment 2 Michael Rennie

2010-08-18 12:22:35 EDT

the following command line works as expected on Linux and Mac, but its Windows equivalent does not load any scripts:

/vms/jdk1.5.0_18/bin/java -cp /home/mrennie/workspaces/workspace/org.mozilla.javascript:/home/mrennie/workspaces/workspace/org.eclipse.wst.jsdt.debug.transport/bin:/home/mrennie/workspaces/workspace/org.eclipse.wst.jsdt.debug.rhino.debugger/bin org.eclipse.wst.jsdt.debug.rhino.debugger.shell.DebugShell -port 37434 -suspend y -version 170 -encoding UTF-8 -opt -1 -e load("/home/mrennie/scripts s/scr.js","/home/mrennie/scripts s/script.js","/home/mrennie/scripts s/fib.js","/home/mrennie/scripts s/script1.js","/home/mrennie/workspaces/target/foo.js.jsproject/again.js")

Comment 3 Michael Rennie

2010-08-18 16:14:28 EDT

Upping priority

Testing some additional escaping it seems like the Rhino parser cannot handle a colon or spaces in the load(..) command. This completely blocks the use of the single-click launch on Windows.

Have to do more testing to find out definitively if this is a Rhino bug or ours.

Comment 4 Michael Rennie

2010-08-18 17:11:46 EDT

supplanting " for ' in the load command makes the complaining about the : in windows paths go away, but Rhino does not seem to be reporting anything.

C:\VMs\SUN-1.5.0.18\bin\javaw.exe -cp "C:\Documents and Settings\MRennie\workspaces\workspace\org.mozilla.javascript";"C:\Documents and Settings\MRennie\workspaces\workspace\org.eclipse.wst.jsdt.debug.transport\bin";"C:\Documents and Settings\MRennie\workspaces\workspace\org.eclipse.wst.jsdt.debug.rhino.debugger\bin" org.eclipse.wst.jsdt.debug.rhino.debugger.shell.DebugShell -port 1997 -suspend y -version 170 -encoding UTF-8 -opt -1 -e load('C:/Documents and Settings/MRennie/workspaces/current_target/usagetests/script.js')

will happily load all of the scripts, but we are not notified of script loads, exceptions, breakpoints, etc from Rhino.

Comment 5 Michael Rennie

2010-08-19 15:33:01 EDT

applied a partial fix to always use ' instead of " in the load command.

Comment 6 Michael Rennie

2010-08-20 15:29:30 EDT

Created attachment 177138 [details]
possible fix

This patch changes the way we launch Rhino to avoid any of the -e <script> weirdness. The patch implements our own proxy to interpret a batch of scripts using the standard -f <script_path> argument from Rhino. It works fantastic on Linux / Mac, but I need to test it on Windows. It also prevents the annoying script loads that Rhino reports from the load(..) command.

Comment 7 Michael Rennie

2010-08-20 15:41:16 EDT

works well on windows as well. 

Applied the patch to HEAD.