Bug 38070

Summary: httpd returns status code 200 instead 304, but logged 304 in log.
Product: Apache httpd-2 Reporter: Masanari Iida <standby24x7>
Component: AllAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: RESOLVED FIXED    
Severity: major CC: Dave.Sparks
Priority: P2    
Version: 2.2.0   
Target Milestone: ---   
Hardware: All   
OS: All   

Description Masanari Iida 2005-12-29 08:42:18 UTC
Version httpd-2.2.0
Use mod_cgi

This symptom has been reported against older apaches before.
apache 1.3 http://archive.apache.org/gnats/5640
apache 2.0 http://issues.apache.org/bugzilla/show_bug.cgi?id=37166
But the solution is not included in current 1.3 and 2.0 sources.
And this time,Ireport it against apache 2.2.0.

Symptom:
Browser access cgi program multiple times.
1st time, httpd returns status code 200 packet and logged status 200
in accesslog. Correct screen is display.
2nd time, httpd returns status code 200 without BODY and logged 
status 304 in accesslog.  Blank screen is display.

Sample script:
http://issues.apache.org/bugzilla/attachment.cgi?id=16757

Fix patchlet:
http://archive.apache.org/gnats/5640

I hope apache developer will include the patch into all 3
version of httpd souce trees.

Regards,
Masanari Iida
Comment 1 Masanari Iida 2006-01-15 07:14:07 UTC
I have posted this symptom to apache users mailing list.
Nick Kew reply to the question.

From: Nick Kew <>	Mailed-By: httpd.apache.org
Reply-To: users httpd apache org
To: users@httpd apache org
Date: Jan 15, 2006 4:26 AM
Subject: Re: [users@httpd] Bug or feature?

On Saturday 14 January 2006 18:04, Masanari Iida wrote:
> Hi,
>
> I would like to ask the list members if following are
> bug or feature of apache.
>
> Use following sample script,
> Apache version: ANY  (1.3, 2.0 and 2.2)
>
> #!/bin/sh
> cat <<EOT
> Status: 200 OK
> Last-Modified: Tue, 15 Feb 2005 15:00:00 GMT
> Content-Type: text/html
>
> Hello world
> EOT

Interesting.  I can confirm that your CGI script with an If-Modified-Since
header later than the Last-Modified date supplied by the script does
indeed return 200 with no body.  That's broken, but is it Apache or
the script that's at fault[1]?

RFC2616 says of If-Modified-Since:

     c) If the variant has not been modified since a valid If-^M
        Modified-Since date, the server SHOULD return a 304 (Not^M
        Modified) response.^M

That makes sense: the script is stupid but technically within its rights
to send the 200 unconditionally.  So Apache should presumably
accommodate it by ignoring the If-Modified-Since header and
returning 200 with the full body.

If that's not already in bugzilla, you might consider entering it there.

[1] It's both, of course.

--
Nick Kew
Comment 2 Nick Kew 2006-01-20 02:38:02 UTC
Fixed in trunk:  r370692 
Comment 3 Dave Sparks 2006-02-02 21:06:08 UTC
Are you sure you don't want to convert a CGI-generated 200 to a 304 when the
HTTP conditions fail?  ap_scan_script_header_err is also called by mod_asis .  I
use mod_asis extensively, with files which include Last-Modified: and ETag:
headers, and it would be disastrous to return 200 when 304 would be appropriate.

Admittedly, I've had to patch both mod_asis.c and util_script.c to get the right
results, but my server seems to return the responses I expect.
Comment 4 Nick Kew 2006-02-03 01:39:45 UTC
(In reply to comment #3) 
> Are you sure you don't want to convert a CGI-generated 200 to a 304 when the 
> HTTP conditions fail? 
 
There are two cases.  If the CGI *explicitly* generates a Status: header, we 
should honour it.  If not, then we just need to generate whatever is 
appropriate - usually 200, or 302 if the CGI emitted a Location header. 
 
>   ap_scan_script_header_err is also called by mod_asis .  I 
 
The crucial difference thare is that mod_asis isn't documented as having a 
Status header (though I guess it might, if it goes through the same parsing as 
CGI). 
 
> use mod_asis extensively, with files which include Last-Modified: and ETag: 
> headers, and it would be disastrous to return 200 when 304 would be 
appropriate. 
 
Your asis doesn't say "Status: foo"?  Then the patch won't affect it. 
>  
> Admittedly, I've had to patch both mod_asis.c and util_script.c to get the 
right 
> results, but my server seems to return the responses I expect. 
 
If you're saying we've got something wrong in the patch, please explain. 
Comment 6 Masanari Iida 2006-02-21 10:49:43 UTC
I have opened Red Hat's bugzilla case and ask RH to back
port your patch into RH's.  
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=176663

Then RH's engineer wrote in comment #5.

>Comment #5 From Joe Orton (jorton@redhat.com)  	 on 2006-02-20 10:49 EST  
[reply]  	   	 
>
>The fix committed upstream prevents handling of conditional requests with a CGI
>script which outputs an explicit (albeit redundant) "Status: 200" header.  This
>would count as a regression so we would not include that patch as-is in a RHEL
>update.
>
>I've prepared a (simpler) alternative patch, which fixes the real issue and will
>make packages available for testing.
>

--- httpd-2.0.52/server/util_script.c.cgistatus
+++ httpd-2.0.52/server/util_script.c
@@ -462,6 +462,13 @@

            if ((cgi_status == HTTP_OK) && (r->method_number == M_GET)) {
                cond_status = ap_meets_conditions(r);
+
+                /* In case an explicit Status: header had set
+                 * r->status_line, then unset it here, so that the
+                 * actual handler return value will be honoured. */
+                if (cond_status != OK) {
+                    r->status_line = NULL;
+                }
            }
            apr_table_overlap(r->err_headers_out, merge,
                APR_OVERLAP_TABLES_MERGE);

With Red Hat's fix, apache with sample cgi return 200,304,304,304.
With Nick's fix, apache with sample cgi return 200,200,200,200.
Both cases, no more white (empty) display.
But,which one is better solution?
Comment 7 Nick Kew 2006-02-21 16:56:08 UTC
The CGI spec. is quite explicit on this: 
 
7.2.1.3. Status   
 The "Status" header field is used to indicate to the server what status code  
the server MUST use in the response message. 
 
so a patch that causes it to change that breaks CGI. 
 
Having said that, for the particular example reported in this bug, Joe's fix 
is better in practical terms.  That's because the CGI script itself misused 
"status".  See todays thread on dev@httpd. 
Comment 8 Dave Sparks 2006-02-21 20:39:24 UTC
Notwithstanding a semantically flawed sentence in a draft which expired over six
years ago, a CGI script which includes cache validation headers in its response
cannot rely on a status code of 200 being returned to the client.  An HTTP/1.1
proxy may return a 304 response without troubling the server; or if it has to
transmit the request via an HTTP/1.0 proxy it may convert a conditional GET to a
HEAD whose 200 response may be converted to a 304 if the conditions are
satisfied.  Insisting that a 200 response with cache validation headers be
transmitted unchanged is futile.

The draft supposedly encodes current CGI practice, but I suspect that in the
area of cache validation headers in CGI-generated responses there is no current
practice to encode.

The documentation for mod_asis says "A Status: header is also required", where
'required' implies that it can never be omitted.  Since the header handling is
common with mod_cgi the documentation is plainly wrong.  I have removed the
Status line from my 802 .asis files with no ill effects.

Whatever the outcome of the argument about "Status: 200" with a "Last-Modified:"
which satisfies a conditional request, I'm going to have to continue to patch
ap_scan_script_header_err to include my "ETag:" headers in the check.
Comment 9 Nick Kew 2006-02-21 22:29:37 UTC
(In response to Dave Sparks) 
 
1. What a proxy can do with a response is totally independent of CGI rules.  
They operate at different levels, and affect different agents. 
 
2. Thanks for the headsup re: mod_asis documentation.  I've just fixed 
the .xml source, so that should propagate through to the live docs in due 
course. 
 
3. You keep telling us you had to patch a problem, but I still can't see a 
problem description.  If we had one, maybe we could fix it, as we have done 
the bug that was clearly and accurately described in this bug report.