Bug 54596

Summary: Relative path functionality truncates last character of configuration parameters preventing connector from working.
Product: Tomcat Connectors Reporter: Brad Cobb <bradc>
Component: isapiAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 1.2.37   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: 2014-12-24_jk_54596_c7.patch

Description Brad Cobb 2013-02-22 06:14:37 UTC
Hi Team,

Found the below issue on the following environment:
  Windows Server 2008 R2 Datacentee (64 bit)
  IIS 7.5 & Tomee 1.5.1
  isapi_redirect.dll 1.2.37 (64 bit)

Issue:  Utilising relative paths in properties file for the various configuration files (log, workers.properties, uriworkermap.properties) truncates last character of the string value.

############################################################
ISAPI_REDIRECT.PROPERTIES
# Configuration file for the Jakarta ISAPI Redirector

# The path to the ISAPI Redirector Extension, relative to the website
# This must be in a virtual directory with execute privileges
extension_uri=/mywebapp/isapi_redirect.dll

# Full path to the log file for the ISAPI Redirector
log_file=jk_iis.log

# Log level (debug, info, warn, error or trace)
log_level=debug 

# Full path to the workers.properties file
worker_file=workers.properties

# Full path to the uriworkermap.properties file
worker_mount_file=uriworkermap.properties
############################################################

... all files do exist in the same C:\Tomee\connectors\ folder.

If I specify absolute paths (C:\Tomee\connectors\uriworkermap.properties), everything works fine.

If I use relative paths with log and config file outside of dll/properties files folder, no log file generated making it difficult to debug.

If I use relative paths with all files in the same folder, a jk_iis.lo (missing the g) file is created with the following output:

[Fri Feb 22 13:57:32.490 2013] [7920:7680] [debug] jk_set_time_fmt::jk_util.c (461): Pre-processed log time stamp format is '[%a %b %d %H:%M:%S.000 %Y] '
[Fri Feb 22 13:57:32.490 2013] [7920:7680] [info] init_jk::jk_isapi_plugin.c (2690): Starting Jakarta/ISAPI/isapi_redirector/1.2.37
[Fri Feb 22 13:57:32.490 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2701): Detected IIS version 7.5
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2703): Using ini file C:\Tomee\connectors\isapi_redirect.properties.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2709): Using log file C:\Tomee\connectors\jk_iis.lo.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2710): Using log level 1.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2711): Using log rotation time 0 seconds.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2712): Using log file size 0 bytes.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2714): Using extension uri /mywebapp/isapi_redirect.dll.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2715): Using worker file C:\Tomee\connectors\workers.propertie.
[Fri Feb 22 13:57:32.505 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2716): Using worker mount file C:\Tomee\connectors\uriworkermap.propertie.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2718): Using rewrite rule file .
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2720): Using uri select 3.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2721): Using no chunked encoding.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2723): Using notification event SF_NOTIFY_AUTH_COMPLETE (0x04000000)
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2733): Using uri header TOMCATURI0000000180000000:.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2734): Using query header TOMCATQUERY0000000180000000:.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2735): Using worker header TOMCATWORKER0000000180000000:.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2736): Using worker index TOMCATWORKERIDX0000000180000000:.
[Fri Feb 22 13:57:32.521 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2737): Using translate header TOMCATTRANSLATE0000000180000000:.
[Fri Feb 22 13:57:32.536 2013] [7920:7680] [debug] init_jk::jk_isapi_plugin.c (2738): Using a default of 250 connections per pool.
[Fri Feb 22 13:57:32.536 2013] [7920:7680] [error] uri_worker_map_load::jk_uri_worker_map.c (1241): Failed to load uri_worker_map file C:\Tomee\connectors\uriworkermap.propertie (errno=2, err=No such file or directory).

Furthermore... if I use the following configuration:

############################################################
ISAPI_REDIRECT.PROPERTIES
# Configuration file for the Jakarta ISAPI Redirector

# The path to the ISAPI Redirector Extension, relative to the website
# This must be in a virtual directory with execute privileges
extension_uri=/mywebapp/isapi_redirect.dll

# Full path to the log file for the ISAPI Redirector
log_file=jk_iis.log

# Log level (debug, info, warn, error or trace)
log_level=debug 

# Full path to the workers.properties file
worker_file=..\conf\workers.properties

# Full path to the uriworkermap.properties file
worker_mount_file=..\conf\uriworkermap.properties
############################################################
  
I get the output:

[Fri Feb 22 14:06:06.888 2013] [7976:8040] [debug] jk_set_time_fmt::jk_util.c (461): Pre-processed log time stamp format is '[%a %b %d %H:%M:%S.000 %Y] '
[Fri Feb 22 14:06:06.888 2013] [7976:8040] [info] init_jk::jk_isapi_plugin.c (2690): Starting Jakarta/ISAPI/isapi_redirector/1.2.37
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2701): Detected IIS version 7.5
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2703): Using ini file C:\Tomee\connectors\isapi_redirect.properties.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2709): Using log file C:\Tomee\connectors\jk_iis.lo.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2710): Using log level 1.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2711): Using log rotation time 0 seconds.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2712): Using log file size 0 bytes.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2714): Using extension uri /mywebapp/isapi_redirect.dll.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2715): Using worker file C:\Tomee\connectors\conf\workers.properties.
[Fri Feb 22 14:06:06.903 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2716): Using worker mount file C:\Tomee\connectors\conf\uriworkermap.properties.
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2718): Using rewrite rule file .
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2720): Using uri select 3.
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2721): Using no chunked encoding.
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2723): Using notification event SF_NOTIFY_AUTH_COMPLETE (0x04000000)
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2733): Using uri header TOMCATURI0000000180000000:.
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2734): Using query header TOMCATQUERY0000000180000000:.
[Fri Feb 22 14:06:06.918 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2735): Using worker header TOMCATWORKER0000000180000000:.
[Fri Feb 22 14:06:06.934 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2736): Using worker index TOMCATWORKERIDX0000000180000000:.
[Fri Feb 22 14:06:06.934 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2737): Using translate header TOMCATTRANSLATE0000000180000000:.
[Fri Feb 22 14:06:06.934 2013] [7976:8040] [debug] init_jk::jk_isapi_plugin.c (2738): Using a default of 250 connections per pool.
[Fri Feb 22 14:06:06.934 2013] [7976:8040] [error] uri_worker_map_load::jk_uri_worker_map.c (1241): Failed to load uri_worker_map file C:\Tomee\connectors\conf\uriworkermap.properties (errno=2, err=No such file or directory).

... whereas the initial declaration of file location is correct but subsequent uri_worker_map load ignores the "..\" relative declaration. 
  
Workaround:  
  1.  Use absolute paths.
  2.  Add a superfluous character in your isapi_redirect.properties file so it is truncated instead of the real value.  e.g.  "uriworkmap.propertiesx"

I hope this enough information to find the issue.  If I can help with other information or testing, let me know.
Thanks
Brad
Comment 1 John Palmer 2014-01-31 16:44:51 UTC
I found the same issue, didn't find this bug, adding comment to help others find it:
on a Windows Server 2008 R2 system with IIS 7.5,
When using the properties FILE rather than Registry entires to configure the connector, and attempting to access a JSP or non-JSP error, the browser gives a HTTP 500 error.

The System Event shows two relevant entries in the Application Log:

Application Log
Source: IIS-W3SVC-WP. Error 2214
The HTTP Filter DLL E:\Program Files\Apache Software Foundation\Jakarta\isapi_redirect.dll failed to load.  The data is the error.

Source: IIS-W3SVC-WP. Error 2268
Could not load all ISAPI filters for site 'GDSCNOLA'.  Therefore site startup aborted.
---------------------------

The root cause appears to be that the code drops the last character of the  file names pulled from the isapi_redirect.properties file, including the log file name generated by this.
This is indicated in the log file generated and CONFIRMED using Process Monitor... which showed failure messages attempting to find these files with the missing last character.
Don't know if this happens for the 32-bit connector or not.
Tried appending a space at the end of those lines with file names, did not help. Nor did enclosing in double-quotes (the parsed file name then started with a double-quote).
Using the Registry for these settings worked fine.

isapi_redirect.properties
---------------------------
extension_uri=/jakarta/isapi_redirect.dll
worker_file=workers.properties.minimal 
worker_mount_file=uriworkermap.properties 
log_level=debug 
log_file=jakarta.log 
---------------------------
Comment 2 Chuck Caldarale 2014-01-31 17:18:14 UTC
(In reply to John Palmer from comment #1)
> on a Windows Server 2008 R2 system with IIS 7.5,
> When using the properties FILE rather than Registry entires to configure the
> connector, and attempting to access a JSP or non-JSP error, the browser
> gives a HTTP 500 error.

Have you checked for Windows-compatible line terminators in each line of the properties file?  If they're not CR-LF, something odd might happen.
Comment 3 Thomas Hoffmann 2014-04-23 15:26:40 UTC
I have also encountered this problem.
Testet with 1.2.40.0 of isapi_redirect.dll
The relative paths are not taken into account. Line-breaks are CR+LF
Neither \ nor / are working. 

Sample Config-File with relevant entries:
log_file=iis_redirect.log
worker_file=..\..\conf\worker.properties
worker_mount_file=..\..\conf\uriworkermap.properties


Logfile is written with the following messages:
[Wed Apr 23 17:20:34.695 2014] [6716:13964] [debug] jk_set_time_fmt::jk_util.c (480): Pre-processed log time stamp format is '[%a %b %d %H:%M:%S.000 %Y] '
[Wed Apr 23 17:20:34.699 2014] [6716:13964] [info] init_jk::jk_isapi_plugin.c (2689): Starting Jakarta/ISAPI/isapi_redirector/1.2.40
[Wed Apr 23 17:20:34.702 2014] [6716:13964] [debug] init_jk::jk_isapi_plugin.c (2700): Detected IIS version 8.0
[Wed Apr 23 17:20:34.705 2014] [6716:13964] [debug] init_jk::jk_isapi_plugin.c (2702): Using ini file D:\Programme\Apache Software Foundation\Tomcat 7.0.53\bin\win32\isapi_redirect.properties.
[Wed Apr 23 17:20:34.707 2014] [6716:13964] [debug] init_jk::jk_isapi_plugin.c (2708): Using log file D:\Programme\Apache Software Foundation\Tomcat 7.0.53\bin\win32\iis_redirect.lo.
...
[Wed Apr 23 17:20:34.724 2014] [6716:13964] [debug] init_jk::jk_isapi_plugin.c (2714): Using worker file D:\Programme\Apache Software Foundation\Tomcat 7.0.53\bin\win32\conf\worker.properties.
[Wed Apr 23 17:20:34.727 2014] [6716:13964] [debug] init_jk::jk_isapi_plugin.c (2715): Using worker mount file D:\Programme\Apache Software Foundation\Tomcat 7.0.53\bin\win32\conf\uriworkermap.properties.
...
[Wed Apr 23 17:20:34.752 2014] [6716:13964] [error] uri_worker_map_load::jk_uri_worker_map.c (1280): Failed to load uri_worker_map file D:\Programme\Apache Software Foundation\Tomcat 7.0.53\bin\win32\conf\uriworkermap.properties (errno=2, err=No such file or directory).


This shows that the dots for the relative paths are truncated and not taken into account. The conversion from relative path to absolute path seems to fail.


Tested under Win 8.1 64 Bit, Connector version 1.2.40.0
Hope this informations helps to narrow down the problem.
Comment 4 Christopher Schultz 2014-04-23 19:27:10 UTC
(In reply to Thomas Hoffmann from comment #3)
> I have also encountered this problem.
> Testet with 1.2.40.0 of isapi_redirect.dll
> The relative paths are not taken into account. Line-breaks are CR+LF
> Neither \ nor / are working. 
> 
> Sample Config-File with relevant entries:
> log_file=iis_redirect.log
> worker_file=..\..\conf\worker.properties
> worker_mount_file=..\..\conf\uriworkermap.properties

[snip]

> init_jk::jk_isapi_plugin.c (2702): Using ini file D:\Programme\Apache
> Software Foundation\Tomcat 7.0.53\bin\win32\isapi_redirect.properties.

This path appears to be correct.

> [Wed Apr 23 17:20:34.707 2014] [6716:13964] [debug]
> init_jk::jk_isapi_plugin.c (2708): Using log file D:\Programme\Apache
> Software Foundation\Tomcat 7.0.53\bin\win32\iis_redirect.lo.

This one appears to not be correct.

Is this only a problem with the log file specifically? Other examples I see in here all have a correct properties file name and a broken log file name.

> ...
> [Wed Apr 23 17:20:34.724 2014] [6716:13964] [debug]
> init_jk::jk_isapi_plugin.c (2714): Using worker file D:\Programme\Apache
> Software Foundation\Tomcat 7.0.53\bin\win32\conf\worker.properties.
> [Wed Apr 23 17:20:34.727 2014] [6716:13964] [debug]
> init_jk::jk_isapi_plugin.c (2715): Using worker mount file
> D:\Programme\Apache Software Foundation\Tomcat
> 7.0.53\bin\win32\conf\uriworkermap.properties.
> ...
> [Wed Apr 23 17:20:34.752 2014] [6716:13964] [error]
> uri_worker_map_load::jk_uri_worker_map.c (1280): Failed to load
> uri_worker_map file D:\Programme\Apache Software Foundation\Tomcat
> 7.0.53\bin\win32\conf\uriworkermap.properties (errno=2, err=No such file or
> directory).
> 
> 
> This shows that the dots for the relative paths are truncated and not taken
> into account. The conversion from relative path to absolute path seems to
> fail.
> 
> 
> Tested under Win 8.1 64 Bit, Connector version 1.2.40.0
> Hope this informations helps to narrow down the problem.

This appears to be a different issue: relative paths aren't properly-resolved at all.
Comment 5 Rainer Jung 2014-12-23 20:09:30 UTC
I have fixed the problem with the missing last character in file names in r1647660. It will be part of version 1.2.41.

The problem only happens, if the configured relativ file path does not contain a ".." path component.

I can also confirm the second part of this bug, namely that ".." path components are not handled correctly, if they would go up the directory hierarchy further than the start of the file name. ".." components which exceed that directory will simply be ignored. So "a/b/c/../../file" would work, but "../file" would result in the same as "file" (except that the other bug with the missing last character would not be triggered).

I will see, whether I can fix this as well.
Comment 6 Rainer Jung 2014-12-23 22:12:03 UTC
I hope I have fixed the second problem - handling ".." path segments - in r1647684. It will be part of version 1.2.41, help with testing welcome.
Comment 7 Konstantin Kolinko 2014-12-24 03:35:10 UTC
Reviewing the code of native/iis/jk_isapi_plugin.c I have the following comments

1). The skip_prefix() utility method can return a non-null value without assigning any values to "sp" and "cp" pointers. It may result in crash in a caller of that method.

Actually the "cp" value is never used by the callers of skip_prefix() method. That argument can be removed.

Essentially, the skip_prefix() method returns two values:

1. The return value (path): Pointer to the first character after 'C:' or '//share/' prefix.
2. sp: "start pointer", "start prefix"?  Pointer to the first valuable character - the first character of 'C:' or '//share/' prefix, or the same as the main return value if there is no prefix.


2). In relative_path():

[[[
                        while (cp > sp) {
]]]

Technically, the minimum value for "cp" is not sp, but the return value of skip_prefix() call. If those are different then it means that the value if an absolute path starting with "C:" or "//share/" prefix.

Actually that never happens in practice, because we never deal with an absolute path here. All calls to path_merge() -> relative_path() are guarded by a "is_path_relative()" condition.


3). In relative_path():

There is a while(nd > 1) loop. It provides support for n consecutive dots, as supported on Unix.  On Windows the code supports only '.' or '..' dots. As such, the loop can be simplified to a simple if(nd > 1).

(If instead of that one wants to implement support for relative paths with more than 2 dots, then
 (*remain)++;
has to be replaced with
 (*remain)+=(nd-1);
)
Comment 8 Konstantin Kolinko 2014-12-24 03:38:02 UTC
Created attachment 32328 [details]
2014-12-24_jk_54596_c7.patch

Patch (untested) to address the issues from Comment 7
Comment 9 Rainer Jung 2014-12-24 08:48:09 UTC
Thanks a bunch Konstantin, I committed the patch in r1647746. Looked good to me.
I plan to do testing next week.
Keeping this issue open until then as a reminder.
Comment 10 Konstantin Kolinko 2014-12-24 11:26:06 UTC
In path_merge()

[[[
            if (remain > 0) {
                return "";
            }
]]]

maybe
                SetLastError(ERROR_BAD_PATHNAME);
                return 0;

like in other erroneous situations.

Callers of path_merge() check its success with a non-null condition. I think that they are not ready to receive an empty string.
Comment 11 Mark Thomas 2016-09-12 19:24:18 UTC
Assuming this is fixed in 1.2.41.