mod_isapi: Ensure we walk through all the methods the developer may have

  employed to report their HTTP status result code.

PR: 16637, 30033
Submitted by: Matt Lewandowsky <matt iamcode.net>
Reviewed by: William Rowe

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@416272 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 11811091b563215e894f24e436597580614b690e..5d09528b2c7d2d263f69e444f93414a531a21e7c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) mod_isapi: Ensure we walk through all the methods the developer may have
+     employed to report their HTTP status result code.
+     PR 16637, 30033.  [Matt Lewandowsky <matt iamcode.net>, William Rowe]
 
   *) New SSLLogLevelDebugDump [ None (default) | IO (not bytes) | Bytes ]
      configures the I/O Dump of SSL traffic, when LogLevel is set to Debug.

diff --git a/modules/arch/win32/mod_isapi.c b/modules/arch/win32/mod_isapi.c
index c7352d2a2f0a9f6bfa0f43350299da0b8638a865..230e70607706ff26ea926eae15321e12a515bdbf 100644 (file)
--- a/modules/arch/win32/mod_isapi.c
+++ b/modules/arch/win32/mod_isapi.c
@@ -630,6 +630,8 @@ static apr_ssize_t send_response_header(isapi_cid *cid,
 {
     int head_present = 1;
     int termarg;
+    int res;
+    int old_status;
     const char *termch;
     apr_size_t ate = 0;
 
@@ -709,29 +711,71 @@ static apr_ssize_t send_response_header(isapi_cid *cid,
      *
      * Parse them out, or die trying
      */
+    old_status = cid->r->status;
+
     if (stat) {
-        cid->r->status = ap_scan_script_header_err_strs(cid->r, NULL,
-                                        &termch, &termarg, stat, head, NULL);
+        res = ap_scan_script_header_err_strs(cid->r, NULL, &termch, &termarg,
+                stat, head, NULL);
+    }
+    else {
+        res = ap_scan_script_header_err_strs(cid->r, NULL, &termch, &termarg,
+                head, NULL);
+    }
+
+    /* Set our status. */
+    if (res) {
+        /* This is an immediate error result from the parser
+         */
+        cid->r->status = res;
+        cid->r->status_line = ap_get_status_line(cid->r->status);
+        cid->ecb->dwHttpStatusCode = cid->r->status;
+    }
+    else if (cid->r->status) {
+        /* We have a status in r->status, so let's just use it.
+         * This is likely to be the Status: parsed above, and
+         * may also be a delayed error result from the parser.
+         * If it was filled in, status_line should also have
+         * been filled in.
+         */
+        cid->ecb->dwHttpStatusCode = cid->r->status;
+    }
+    else if (cid->ecb->dwHttpStatusCode
+              && cid->ecb->dwHttpStatusCode != HTTP_OK) {
+        /* Now we fall back on dwHttpStatusCode if it appears
+         * ap_scan_script_header fell back on the default code.
+         * Any other results set dwHttpStatusCode to the decoded
+         * status value.
+         */
+        cid->r->status = cid->ecb->dwHttpStatusCode;
+        cid->r->status_line = ap_get_status_line(cid->r->status);
+    }
+    else if (old_status) {
+        /* Well... either there is no dwHttpStatusCode or it's HTTP_OK.
+         * In any case, we don't have a good status to return yet...
+         * Perhaps the one we came in with will be better. Let's use it,
+         * if we were given one (note this is a pendantic case, it would
+         * normally be covered above unless the scan script code unset
+         * the r->status). Should there be a check here as to whether
+         * we are setting a valid response code?
+         */
+        cid->r->status = old_status;
+        cid->r->status_line = ap_get_status_line(cid->r->status);
         cid->ecb->dwHttpStatusCode = cid->r->status;
     }
     else {
-        cid->r->status = ap_scan_script_header_err_strs(cid->r, NULL,
-                                        &termch, &termarg, head, NULL);
-        if (cid->ecb->dwHttpStatusCode && cid->r->status == HTTP_OK
-                && cid->ecb->dwHttpStatusCode != HTTP_OK) {
-            /* We tried every way to Sunday to get the status...
-             * so now we fall back on dwHttpStatusCode if it appears
-             * ap_scan_script_header fell back on the default code.
-             * Any other results set dwHttpStatusCode to the decoded
-             * status value.
-             */
-            cid->r->status = cid->ecb->dwHttpStatusCode;
-            cid->r->status_line = ap_get_status_line(cid->r->status);
-        }
-        else {
-            cid->ecb->dwHttpStatusCode = cid->r->status;
-        }
+        /* None of dwHttpStatusCode, the parser's r->status nor the 
+         * old value of r->status were helpful, and nothing was decoded
+         * from Status: string passed to us.  Let's just say HTTP_OK 
+         * and get the data out, this was the isapi dev's oversight.
+         */
+        cid->r->status = HTTP_OK;
+        cid->r->status_line = ap_get_status_line(cid->r->status);
+        cid->ecb->dwHttpStatusCode = cid->r->status;
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, cid->r,
+                "ISAPI: Could not determine HTTP response code; using %d",
+                cid->r->status);
     }
+
     if (cid->r->status == HTTP_INTERNAL_SERVER_ERROR) {
         return -1;
     }