Item10438: lighttpd incompatibility - found in viewfile, but could be there for other things too

pathinfo and redirect infos are quite different among http servers. lighttpd does it yet differently ... which triggers this error.

Situation is that actually all scripts are accessed as part of a redirect. Viewfile now thinks this must be a 4XX error page. Infact the REQUEST_STATUS is a good one namely 200. At this point the filename parameter is not read which results in an attachment-not-found oops.

This patch seems to fix it. Can somebody confirm?

--- lib/Foswiki/UI/Viewfile.pm  (revision 10878)
+++ lib/Foswiki/UI/Viewfile.pm  (working copy)
@@ -44,7 +44,7 @@
     my $fileName;
     my $pathInfo;
 
-    if ( defined( $ENV{REDIRECT_STATUS} ) && defined( $ENV{REQUEST_URI} ) ) {
+    if ( defined( $ENV{REDIRECT_STATUS} ) && $ENV{REDIRECT_STATUS} != 200 && defined( $ENV{REQUEST_URI} ) ) {

        # this is a redirect - can be used to make 404,401 etc URL's
        # more foswiki tailored and is also used in TWikiCompatibility
        $pathInfo = $ENV{REQUEST_URI};

        # ignore parameters, as apache would.
        $pathInfo =~ s/^(.*)(\?|#).*/$1/;
        $pathInfo =~ s|$Foswiki::cfg{PubUrlPath}||;    #remove pubUrlPath

-- MichaelDaum - 03 Mar 2011

Anybody running lighttpd that can confirm?

Has the patch been tested on Apache?

You did not set target release. You want this to block 1.1.3?

-- KennethLavrsen - 06 Mar 2011

Added some more knowledgeable people on the waiting for list.

-- MichaelDaum - 08 Mar 2011

Patch looks reasonable to me.

-- CrawfordCurrie - 13 Mar 2011

this does not fix viewfile enough

for eg, on apache

• http://foswiki.org/bin/viewfile/System/ProjectLogos/foswiki-logo.gif

works, but it doesn't on lighttpd - it complains that ? attachment does not exist.

Michael - I'm using the core/toold/lighttpd.pl to test, as i'd like to avoid the time to learn all of lighttpd atm (i'd rather learn nginx, but don't really have time for that either)

can you please give me a config file for lighttpd to reproduce your issue? or better yet, patch the lighttpd.pl in trunk?

-- SvenDowideit - 17 Mar 2011

Hi Sven, attached my config. The ? attachment does not exist is exactly what the above patch was fixing for me.

-- MichaelDaum - 17 Mar 2011

ta - it doesn't fix it for me, but I was testing the non-redirect version. I'll play with it this weekend.

-- SvenDowideit - 18 Mar 2011

I ended up not having time to deal with this yet, so there's no reason to block the 1.1.3 release. bummer, but thats life.

-- SvenDowideit - 24 Mar 2011

Could anybody shed some light on what exactly the affected code area is all about? It seems as if it is only used to implement error redirect pages or something ... so checking the http status is actually a must, isnt it?

After long thinking I came to the conclusion that this if branch checking for a redirect is quite useless. Instead of fixing a feature that nobody seems to be able to explain, which is buggy and probably not documented anyway the more sensible solution would be to remove this if branch all together.

... which under any circumstances is a release blocker.

-- MichaelDaum - 24 Mar 2011

I don't understand how this is a release blocker unless there is a test case, either in the TestCases, or a live example that shows that it is broken. Everything works fine as far as I can tell on Apache. In any event, making a change in this area without a test case to prove correct operation this late in the release cycle is very risky. There are a number of very critical fixes, including some effecting security, being held up for something without a demonstrated failure on Apache or UnitTests.

Please consider creating a test case so that the before/after impact of the change can be verified.

I've added the comments from the code to the above example. So this change needs to be tested thorough on an apache system running with redirect to viewfile, for both found and not-found files. It appears that it would only apply to 4xx codes, but it needs to be tested.

-- GeorgeClark - 24 Mar 2011

Actually reading our InstallationGuide, and release notes, I don't see where Lighttpd is a supported environment. Doesn't this need a feature proposal to add lighttpd as a supported environment complete with appropriate documentation.

-- GeorgeClark - 24 Mar 2011

I have not seen any description of an error mode that will prevent a patch release

You can set this to urgent or very urgent or deadly. 1.1.3 is so much better than 1.1.2 that it is about time to get it out. And the already fixed bugs are more urgent to get out than fixing this corner case that 99% of us cannot even reproduce.

If this was so bloody urgent it would have been fixed urgently when it was raised. No matter what state this item has 1.1.3 is going out when the major languages are back at near 100%

-- KennethLavrsen - 24 Mar 2011

George, from what I see these lines of code are simply bit rot with no real value at all. No surprise there isn't even a unit test for the code as is.

-- MichaelDaum - 25 Mar 2011

All this means is that yes, we can't rush it, cos we need to understand it before we change it, and then test all the cases it was intended for - clearly not a rushjob.

-- SvenDowideit - 25 Mar 2011

Commenting is disabled while not logged in