Bug 40817 - servlet-cgi throws index out of bounds exception on certain cgi
servlet-cgi throws index out of bounds exception on certain cgi
Status: RESOLVED FIXED
Product: Tomcat 5
Classification: Unclassified
Component: Servlets:CGI
5.5.20
PC Linux
: P2 major (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2006-10-25 20:02 UTC by Gene Hardesty
Modified: 2006-10-27 17:52 UTC (History)
0 users



Attachments
version of CGIServlet.java that doesn't have the out-of-bounds bug (70.67 KB, text/plain)
2006-10-25 20:06 UTC, Gene Hardesty
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gene Hardesty 2006-10-25 20:02:52 UTC
First, servlets-cgi.jar needs to exist and the servlet must be mapped:
for example:
 <servlet-mapping>
   <servlet-name>cgi</servlet-name>
   <url-pattern>*.pl</url-pattern>
 </servlet-mapping>

Test perl cgi script:
 #!/usr/bin/perl
 print "Content-type: text/plain", "\n\n";
 print "Hello world, from Perl\n";

One perl script is placed in www-root ("/test.pl") and the other in any
subfolder ("/test/test.pl" or "/cgi-bin/test.pl", etc.)

The cgi script in the subfolder will run fine.
The cgi script in the www-root will generate an exception:
java.lang.StringIndexOutOfBoundsException: String index out of range: -1
	java.lang.String.substring(String.java:1768)
	java.lang.String.substring(String.java:1735)
	org.apache.catalina.servlets.CGIServlet$CGIEnvironment.findCGI(CGIServlet.java:948)
	org.apache.catalina.servlets.CGIServlet$CGIEnvironment.setCGIEnvironment(CGIServlet.java:1015)
	org.apache.catalina.servlets.CGIServlet$CGIEnvironment.<init>(CGIServlet.java:766)
	org.apache.catalina.servlets.CGIServlet.doGet(CGIServlet.java:584)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:689)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:802)


The problem is this line (line 948 in CGIServlet.java):

cginame =              
currentLocation.getParent().substring(webAppRootDir.length())+ File.separator +
name;

The problem is that webAppRootDir is 1 char longer than
currentLocation.getParent() because webAppRootDir ends with a File.separator (in
this case, a "/").  And removing the extra File.separator from webAppRootDir
will result in a different String related exception elsewhere.

And here's a fix that works:

cginame = (currentLocation.getParent() + 
File.separator).substring(webAppRootDir.length()) + name;

(If necessary, the fix can be made more adaptive by checking to see if it really
does have a File.separator at the end and if not...add it...otherwise, let it
be, etc.)

(on a side note, I have one more bug to fix; the ENV_VAR "SCRIPT_FILENAME" isn't
defined as it should be [required for PHP4/5] but I'll submit a different
bug/fix for that.)
Comment 1 Gene Hardesty 2006-10-25 20:06:18 UTC
Created attachment 19040 [details]
version of CGIServlet.java that doesn't have the out-of-bounds bug
Comment 2 Chris Halstead 2006-10-26 12:59:21 UTC
This is likely a configuration issue.  Make sure that you aren't defining the
CGIServlet init-param cgiPathPrefix as '/' in your CGI servlet definition. 
Remove the cgiPathPrefix init-param and it should work as expected.  Setting
cgiPathPrefix to '/' was the only way that I could repro this issue.  By having
that set you are unneccessarily adding an extra '/' to the path:

INFO http-8080-Processor25
org.apache.catalina.core.ContainerBase.[Catalina].[localhost].[/cgi-test] - cgi:
findCGI: path=/test.pl, /home/chris/apache-tomcat-5.5.20/webapps/cgi-test//

The CGIServlet is already set up to trim any trailing file seperator from the
webAppRootDir, but it only trims one:

    if ((webAppRootDir != null)
        && (webAppRootDir.lastIndexOf(File.separator) ==
            (webAppRootDir.length() - 1))) {
            //strip the trailing "/" from the webAppRootDir
            webAppRootDir =
            webAppRootDir.substring(0, (webAppRootDir.length() - 1));
    }

A possibly more appropriate patch would trim an arbitrary number of file
seperators from webAppRootDir, though right now I can't think of another case
where that would be needed.
Comment 3 Gene Hardesty 2006-10-26 14:08:20 UTC
Well, I double-check the init-param but it doesn't have a '/'
It's "blank"
          <param-name>cgiPathPrefix</param-name>
          <param-value></param-value>
(also though it's a different issue, I found that "SCRIPT_NAME" was wrong
too...it was returning "/test.pltest.pl" or "/test/test.pltest/test.pl"...fixed
it in the if statement a few lines down...scriptname = cginame and scriptname =
contextpath + cginame, respectively but that's another issue).

wait....I see....line 918 adds an extra '/' if the pathprefix setting is null.

anyways, "cginame = (currentLocation.getParent() +
File.separator).substring(webAppRootDir.length()) + name;" seems to work.


btw, how do I submit a feature request? (I added it myself as I was having
problems with PHP CGI....I made it so under certain circumstances, it will use
"php" instead of "perl" as the cgiexecutable and lo-and-behold, it
works...though I also had to add the env "SCRIPT_FILENAME" [which is just a
exact copy of "X_TOMCAT_SCRIPT_PATH"] ).  I plan to make the php "enhancement"
as a part of the init-param so it can be turned on or off as need be (as well as
be able to define what constitutes "PHP" mode...as right now, it's hard-coded to
look for commands that end with ".php" ".php3" ".php4" ".phps")

anyways..maybe a regex for removing the trailing '/' might do...

(In reply to comment #2)
> This is likely a configuration issue.  Make sure that you aren't defining the
> CGIServlet init-param cgiPathPrefix as '/' in your CGI servlet definition. 
> Remove the cgiPathPrefix init-param and it should work as expected.  Setting
> cgiPathPrefix to '/' was the only way that I could repro this issue.  By having
> that set you are unneccessarily adding an extra '/' to the path:
> 
> INFO http-8080-Processor25
> org.apache.catalina.core.ContainerBase.[Catalina].[localhost].[/cgi-test] - cgi:
> findCGI: path=/test.pl, /home/chris/apache-tomcat-5.5.20/webapps/cgi-test//
> 
> The CGIServlet is already set up to trim any trailing file seperator from the
> webAppRootDir, but it only trims one:
> 
>     if ((webAppRootDir != null)
>         && (webAppRootDir.lastIndexOf(File.separator) ==
>             (webAppRootDir.length() - 1))) {
>             //strip the trailing "/" from the webAppRootDir
>             webAppRootDir =
>             webAppRootDir.substring(0, (webAppRootDir.length() - 1));
>     }
> 
> A possibly more appropriate patch would trim an arbitrary number of file
> seperators from webAppRootDir, though right now I can't think of another case
> where that would be needed.
Comment 4 Mark Thomas 2006-10-27 17:42:21 UTC
I have fixed the bug and added the PHP environment variable for good measure.

You shouldn't need to hack the CGI servlet now. Just declare it twice, with a
different name, and you can have one set of init-params and mappings for PHP and
another for Perl. In fact, you can have as many as you want.
Comment 5 Gene Hardesty 2006-10-27 17:52:44 UTC
I had to "hack" it for php though.
By default, the cgiexecutable is "perl" but if it tries to run a php file, it
doesn't work.
If the cgiexecutable used for php files is "php" instead of the default perl, it
works flawlessly.
(though wish it had a "run as user" ["su -l user"] option to run the cgi as
something different than tomcat).

(In reply to comment #4)
> I have fixed the bug and added the PHP environment variable for good measure.
> 
> You shouldn't need to hack the CGI servlet now. Just declare it twice, with a
> different name, and you can have one set of init-params and mappings for PHP and
> another for Perl. In fact, you can have as many as you want.