You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2005/06/20 19:27:49 UTC

svn commit: r191517 - /httpd/httpd/branches/1.3.x/src/main/http_protocol.c

Author: wrowe
Date: Mon Jun 20 10:27:48 2005
New Revision: 191517

URL: http://svn.apache.org/viewcvs?rev=191517&view=rev
Log:

  These failure cases are all essentially bogus submissions to httpd,
  do not persist the connection if the client can't formulate any
  respectible request (e.g. likely to be exploit testing).

  [None of the modified failure cases occur prior to request processing.]

Modified:
    httpd/httpd/branches/1.3.x/src/main/http_protocol.c

Modified: httpd/httpd/branches/1.3.x/src/main/http_protocol.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/1.3.x/src/main/http_protocol.c?rev=191517&r1=191516&r2=191517&view=diff
==============================================================================
--- httpd/httpd/branches/1.3.x/src/main/http_protocol.c (original)
+++ httpd/httpd/branches/1.3.x/src/main/http_protocol.c Mon Jun 20 10:27:48 2005
@@ -1186,6 +1186,7 @@
 
             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
                          "request failed: URI too long");
+            r->connection->keepalive = 0;
             ap_send_error_response(r, 0);
             ap_log_transaction(r);
             return r;
@@ -1194,6 +1195,7 @@
             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
                          "request failed: erroneous characters after protocol string: %s",
 			 ap_escape_logitem(r->pool, r->the_request));
+            r->connection->keepalive = 0;
             ap_send_error_response(r, 0);
             ap_log_transaction(r);
             return r;
@@ -1207,6 +1209,7 @@
         if (r->status != HTTP_REQUEST_TIME_OUT) {
             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
                          "request failed: error reading the headers");
+            r->connection->keepalive = 0;
             ap_send_error_response(r, 0);
             ap_log_transaction(r);
             return r;
@@ -1260,6 +1263,7 @@
                       "(see RFC2616 section 14.23): %s", r->uri);
     }
     if (r->status != HTTP_OK) {
+        r->connection->keepalive = 0;
         ap_send_error_response(r, 0);
         ap_log_transaction(r);
         return r;



Re: svn commit: r191517 - /httpd/httpd/branches/1.3.x/src/main/http_protocol.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
I'd appreciate another two +1's for this patch I posted some time
ago, I discovered the problem with 1.3's mod_ssl, where we never
set up the connection (the headers were never handshaked correctly)
so... ...the next connection attempt on a keepalive threw up sort
of violently.

I can't think of any justification for keeping alive a connection
under the pre-request failure cases.

Votes/Notes?

Bill

At 12:27 PM 6/20/2005, wrowe@apache.org wrote:
>Author: wrowe
>Date: Mon Jun 20 10:27:48 2005
>New Revision: 191517
>
>URL: http://svn.apache.org/viewcvs?rev=191517&view=rev
>Log:
>
>  These failure cases are all essentially bogus submissions to httpd,
>  do not persist the connection if the client can't formulate any
>  respectible request (e.g. likely to be exploit testing).
>
>  [None of the modified failure cases occur prior to request processing.]
>
>Modified:
>    httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>
>Modified: httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/1.3.x/src/main/http_protocol.c?rev=191517&r1=191516&r2=191517&view=diff
>==============================================================================
>--- httpd/httpd/branches/1.3.x/src/main/http_protocol.c (original)
>+++ httpd/httpd/branches/1.3.x/src/main/http_protocol.c Mon Jun 20 10:27:48 2005
>@@ -1186,6 +1186,7 @@
> 
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: URI too long");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1194,6 +1195,7 @@
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: erroneous characters after protocol string: %s",
>                        ap_escape_logitem(r->pool, r->the_request));
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1207,6 +1209,7 @@
>         if (r->status != HTTP_REQUEST_TIME_OUT) {
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: error reading the headers");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1260,6 +1263,7 @@
>                       "(see RFC2616 section 14.23): %s", r->uri);
>     }
>     if (r->status != HTTP_OK) {
>+        r->connection->keepalive = 0;
>         ap_send_error_response(r, 0);
>         ap_log_transaction(r);
>         return r;



[vote] svn commit: r191517

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
This was lazy concensus; I would prefer an up-down vote.

I can't picture a scenario where, if do not reach a single 
module hook, we want the server to keep the connection open.  
Comments/notes/votes please.

This happened to fix some ugly side effects of a previously
reported mod_ssl bug, but that's only a secondary motivation.

If we concur this is goodness, I'll port to 2.1, then 2.0.

Bill  

At 12:27 PM 6/20/2005, wrowe@apache.org wrote:
>Author: wrowe
>Date: Mon Jun 20 10:27:48 2005
>New Revision: 191517
>
>URL: http://svn.apache.org/viewcvs?rev=191517&view=rev
>Log:
>
>  These failure cases are all essentially bogus submissions to httpd,
>  do not persist the connection if the client can't formulate any
>  respectible request (e.g. likely to be exploit testing).
>
>  [None of the modified failure cases occur prior to request processing.]
>
>Modified:
>    httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>
>Modified: httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/1.3.x/src/main/http_protocol.c?rev=191517&r1=191516&r2=191517&view=diff
>==============================================================================
>--- httpd/httpd/branches/1.3.x/src/main/http_protocol.c (original)
>+++ httpd/httpd/branches/1.3.x/src/main/http_protocol.c Mon Jun 20 10:27:48 2005
>@@ -1186,6 +1186,7 @@
> 
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: URI too long");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1194,6 +1195,7 @@
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: erroneous characters after protocol string: %s",
>                        ap_escape_logitem(r->pool, r->the_request));
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1207,6 +1209,7 @@
>         if (r->status != HTTP_REQUEST_TIME_OUT) {
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: error reading the headers");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1260,6 +1263,7 @@
>                       "(see RFC2616 section 14.23): %s", r->uri);
>     }
>     if (r->status != HTTP_OK) {
>+        r->connection->keepalive = 0;
>         ap_send_error_response(r, 0);
>         ap_log_transaction(r);
>         return r;



Re: svn commit: r191517 - /httpd/httpd/branches/1.3.x/src/main/http_protocol.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
I'd appreciate another two +1's for this patch I posted some time
ago, I discovered the problem with 1.3's mod_ssl, where we never
set up the connection (the headers were never handshaked correctly)
so... ...the next connection attempt on a keepalive threw up sort
of violently.

I can't think of any justification for keeping alive a connection
under the pre-request failure cases.

Votes/Notes?

Bill

At 12:27 PM 6/20/2005, wrowe@apache.org wrote:
>Author: wrowe
>Date: Mon Jun 20 10:27:48 2005
>New Revision: 191517
>
>URL: http://svn.apache.org/viewcvs?rev=191517&view=rev
>Log:
>
>  These failure cases are all essentially bogus submissions to httpd,
>  do not persist the connection if the client can't formulate any
>  respectible request (e.g. likely to be exploit testing).
>
>  [None of the modified failure cases occur prior to request processing.]
>
>Modified:
>    httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>
>Modified: httpd/httpd/branches/1.3.x/src/main/http_protocol.c
>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/1.3.x/src/main/http_protocol.c?rev=191517&r1=191516&r2=191517&view=diff
>==============================================================================
>--- httpd/httpd/branches/1.3.x/src/main/http_protocol.c (original)
>+++ httpd/httpd/branches/1.3.x/src/main/http_protocol.c Mon Jun 20 10:27:48 2005
>@@ -1186,6 +1186,7 @@
> 
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: URI too long");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1194,6 +1195,7 @@
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: erroneous characters after protocol string: %s",
>                        ap_escape_logitem(r->pool, r->the_request));
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1207,6 +1209,7 @@
>         if (r->status != HTTP_REQUEST_TIME_OUT) {
>             ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
>                          "request failed: error reading the headers");
>+            r->connection->keepalive = 0;
>             ap_send_error_response(r, 0);
>             ap_log_transaction(r);
>             return r;
>@@ -1260,6 +1263,7 @@
>                       "(see RFC2616 section 14.23): %s", r->uri);
>     }
>     if (r->status != HTTP_OK) {
>+        r->connection->keepalive = 0;
>         ap_send_error_response(r, 0);
>         ap_log_transaction(r);
>         return r;