You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2020/07/15 14:17:17 UTC

svn commit: r1879891 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS modules/http2/h2_from_h1.c modules/http2/h2_task.c

Author: minfrin
Date: Wed Jul 15 14:17:17 2020
New Revision: 1879891

URL: http://svn.apache.org/viewvc?rev=1879891&view=rev
Log:
 *) mod_http2: Avoid segfaults in case of handling certain responses for
    already aborted connections.
    Trunk version of patch:
      http://svn.apache.org/r1879544
      http://svn.apache.org/r1879546
      http://svn.apache.org/r1879547
    Backport version for 2.4.x of patch:
      https://github.com/apache/httpd/pull/132.diff
    +1: rpluem, icing, minfrin

Modified:
    httpd/httpd/branches/2.4.x/CHANGES
    httpd/httpd/branches/2.4.x/STATUS
    httpd/httpd/branches/2.4.x/modules/http2/h2_from_h1.c
    httpd/httpd/branches/2.4.x/modules/http2/h2_task.c

Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1879891&r1=1879890&r2=1879891&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Wed Jul 15 14:17:17 2020
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.44
 
+  *) mod_http2: Avoid segfaults in case of handling certain responses for
+     already aborted connections.  [Stefan Eissing, Ruediger Pluem]
+
   *) mod_http2: The module now handles master/secondary connections and has marked
      methods according to use. [Stefan Eissing]
 

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1879891&r1=1879890&r2=1879891&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Wed Jul 15 14:17:17 2020
@@ -135,15 +135,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
- *) mod_http2: Avoid segfaults in case of handling certain responses for
-    already aborted connections.
-    Trunk version of patch:
-      http://svn.apache.org/r1879544
-      http://svn.apache.org/r1879546
-      http://svn.apache.org/r1879547
-    Backport version for 2.4.x of patch:
-      https://github.com/apache/httpd/pull/132.diff
-    +1: rpluem, icing, minfrin
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_from_h1.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_from_h1.c?rev=1879891&r1=1879890&r2=1879891&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_from_h1.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_from_h1.c Wed Jul 15 14:17:17 2020
@@ -315,6 +315,7 @@ typedef struct h2_response_parser {
     int http_status;
     apr_array_header_t *hlines;
     apr_bucket_brigade *tmp;
+    apr_bucket_brigade *saveto;
 } h2_response_parser;
 
 static apr_status_t parse_header(h2_response_parser *parser, char *line) {
@@ -351,13 +352,17 @@ static apr_status_t get_line(h2_response
         parser->tmp = apr_brigade_create(task->pool, task->c->bucket_alloc);
     }
     status = apr_brigade_split_line(parser->tmp, bb, APR_BLOCK_READ, 
-                                    HUGE_STRING_LEN);
+                                    len);
     if (status == APR_SUCCESS) {
         --len;
         status = apr_brigade_flatten(parser->tmp, line, &len);
         if (status == APR_SUCCESS) {
             /* we assume a non-0 containing line and remove trailing crlf. */
             line[len] = '\0';
+            /*
+             * XXX: What to do if there is an LF but no CRLF?
+             *      Should we error out?
+             */
             if (len >= 2 && !strcmp(H2_CRLF, line + len - 2)) {
                 len -= 2;
                 line[len] = '\0';
@@ -367,10 +372,47 @@ static apr_status_t get_line(h2_response
                               task->id, line);
             }
             else {
+                apr_off_t brigade_length;
+
+                /*
+                 * If the brigade parser->tmp becomes longer than our buffer
+                 * for flattening we never have a chance to get a complete
+                 * line. This can happen if we are called multiple times after
+                 * previous calls did not find a H2_CRLF and we returned
+                 * APR_EAGAIN. In this case parser->tmp (correctly) grows
+                 * with each call to apr_brigade_split_line.
+                 *
+                 * XXX: Currently a stack based buffer of HUGE_STRING_LEN is
+                 * used. This means we cannot cope with lines larger than
+                 * HUGE_STRING_LEN which might be an issue.
+                 */
+                status = apr_brigade_length(parser->tmp, 0, &brigade_length);
+                if ((status != APR_SUCCESS) || (brigade_length > len)) {
+                    ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, task->c, APLOGNO(10257)
+                                  "h2_task(%s): read response, line too long",
+                                  task->id);
+                    return APR_ENOSPC;
+                }
                 /* this does not look like a complete line yet */
                 ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, task->c,
                               "h2_task(%s): read response, incomplete line: %s", 
                               task->id, line);
+                if (!parser->saveto) {
+                    parser->saveto = apr_brigade_create(task->pool,
+                                                        task->c->bucket_alloc);
+                }
+                /*
+                 * Be on the save side and save the parser->tmp brigade
+                 * as it could contain transient buckets which could be
+                 * invalid next time we are here.
+                 *
+                 * NULL for the filter parameter is ok since we
+                 * provide our own brigade as second parameter
+                 * and ap_save_brigade does not need to create one.
+                 */
+                ap_save_brigade(NULL, &(parser->saveto), &(parser->tmp),
+                                parser->tmp->p);
+                APR_BRIGADE_CONCAT(parser->tmp, parser->saveto);
                 return APR_EAGAIN;
             }
         }

Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_task.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_task.c?rev=1879891&r1=1879890&r2=1879891&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_task.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_task.c Wed Jul 15 14:17:17 2020
@@ -380,7 +380,7 @@ static apr_status_t h2_filter_parse_h1(a
     /* There are cases where we need to parse a serialized http/1.1 
      * response. One example is a 100-continue answer in serialized mode
      * or via a mod_proxy setup */
-    while (bb && !task->output.sent_response) {
+    while (bb && !task->c->aborted && !task->output.sent_response) {
         status = h2_from_h1_parse_response(task, f, bb);
         ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, f->c,
                       "h2_task(%s): parsed response", task->id);