Bug 59065

Summary: catalina.sh should check for colon _after_ cygwin path sanitization
Product: Tomcat 8 Reporter: Ed Randall <ed.randall>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 8.0.32   
Target Milestone: ----   
Hardware: PC   
OS: All   
Attachments: patch for this issue

Description Ed Randall 2016-02-24 18:07:02 UTC
Created attachment 33590 [details]
patch for this issue

Near the top of catalina.sh are a couple of case statements which check for a colon within the CATALINA_HOME and CATALINA_BASE environment variables.

Just after this is a section which sanitizes those same variables to UNIX format, conditional to running under Cygwin.

The Cygwin section should come before the colon check.  
In a Cygwin environment it's entirely likely that CATALINA_HOEM and/or CATALINA_BASE will be set up using Windows semantics, which would include a colon.

The user is then be unable to start/stop etc.

Patch attached.
Comment 1 Christopher Schultz 2016-02-24 21:28:18 UTC
I'm curious, what is the result of using this patch? If Cygwin is being used on Windows with CATALINA_BASE or CATALINA_HOME (or both) starting with a drive letter (this is presumably the use-case, here), then the CLASSPATH will be built with an embedded colon character.

Under this environment, how does the JVM interpret the CLASSPATH environment variable? Does it use the UNIX-style path separator (colon) or does it use the Windows-style path separator (semicolon)? I would imagine that Windows-based JVMs /always/ use semicolons as the path separator, regardless of the use of Cygwin or any other UNIX-like environment running inside of it.

In that case, shouldn't the colon-checking be replaced with semicolon-checking for Windows? The real issue is that a .sh file is being used on Windows instead of the .bat file (which is exclusively for Windows).

I have only looked at the raw diff and not yet in context, but it sounds like moving the check is not appropriate for Cygwin-on-Windows. Changing the check to look for semicolon is the right fix.

Have I misunderstood?
Comment 2 Ed Randall 2016-02-24 22:40:59 UTC
I think you are perhaps unfamiliar with cygwin and have misunderstood.

"cygpath --unix ${VARIABLE}" transforms all drive letters, slashes, colons and semicolons into a unix-style which cygwin and bash will work with correctly.  For example if 
    CATALINA_HOME=C:\apps\apache-tomcat-8.0.32 
then it will become 
    /cygdrive/c/apps/apache-tomcat-8.0.32 
after the transformation. 

Unfortunately these new lines which detect "colon" in these base variables (and  have apparently been added since Tomcat 7) have been placed before the cygwin magic, so in my environment the script bombs immediately on seeing the "C:".  The fix is to simply move them after cygwin-specific transforms.  All is then well.  

In a non-cygwin environment the net effect of the patch is no change.

CLASSPATH is not an issue here. 
Thanks :)
Comment 3 Mark Thomas 2016-03-02 10:50:02 UTC
Thanks for the report and the patch.

I've tested this locally and I can confirm both the problem and the fix.

The patch has been applied to 9.0.x for 9.0.0.M4 onwards nd 8.0.x for 8.0.33 onwards.