1.3 ap_die() is simpler, so use that as an example when referring to code.

case 1:

ErrorDocument 413 /index.html
module returns 413 from handler and doesn't set r->status to 413
GOOD: /index.html is used for the error document

case 2:

ErrorDocument 413 /index.html
module returns 413 from handler and does set r->status to 413
BAD: this is what client sees:
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<HTML><HEAD>
<TITLE>413 Request Entity Too Large</TITLE>
</HEAD><BODY>
<H1>Request Entity Too Large</H1>
The requested resource<BR>/silly<BR>
does not allow request data with GET requests, or the amount of data provided in
the request exceeds the capacity limit.
<P>Additionally, a 413 Request Entity Too Large
error was encountered while trying to use an ErrorDocument to handle the request.
<HR>
<ADDRESS>Apache/1.3.29-dev Server at cs390-1.raleigh.ibm.com Port 80</ADDRESS>
</BODY></HTML>


ap_die() is definitely confused about whether or not it is a recursive error. Any time r->status is set, it thinks it is a recursive error.

Attached is a patch to 1.3's ap_die() to make it smarter about recognizing recursive errors. I suspect that some funny redirect optimizations in 2.0 would make the same fix impossible, but I haven't checked.

On a side note, is it bad for modules to set r->status? Is that why this recursion check, present since 1.2, is not causing grief for others?

Index: src/main/http_request.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_request.c,v
retrieving revision 1.170
diff -u -r1.170 http_request.c
--- src/main/http_request.c     7 Jul 2003 00:34:10 -0000       1.170
+++ src/main/http_request.c     14 Oct 2003 20:01:42 -0000
@@ -1073,13 +1073,16 @@
      */

     if (r->status != HTTP_OK) {
-        recursive_error = type;

-        while (r->prev && (r->prev->status != HTTP_OK))
+        while (r->prev && (r->prev->status != HTTP_OK)) {
+            recursive_error = type;
             r = r->prev;        /* Get back to original error */
+        }

-        type = r->status;
-        custom_response = NULL; /* Do NOT retry the custom thing! */
+        if (recursive_error) {
+            type = r->status;
+            custom_response = NULL; /* Do NOT retry the custom thing! */
+        }
     }

     r->status = type;

Reply via email to