From: Stefan Eissing <icing@apache.org>
Date: Wed, 15 Jun 2016 10:06:21 +0000 (+0000)
Subject: Merge of r1748531 from trunk:
X-Git-Tag: 2.4.21~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=595fa547e864e781626658314e5b3ac60fdb1c0c;p=thirdparty%2Fapache%2Fhttpd.git

Merge of r1748531 from trunk:

mod_http2: more rigid error handling in DATA frame assembly



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1748536 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 7e8d8ab2f73..0454f20901d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.4.21
 
+  *) mod_http2: more rigid error handling in DATA frame assembly, leading
+     to deterministic connection errors if assembly fails.
+     [Stefan Eissing, Pal Nilsen <https://github.com/maedox>]
+
   *) abs: Include OPENSSL_Applink when compiling on Windows, to resolve
      failures under Visual Studio 2015 and other mismatched MSVCRT flavors.
      PR59630 [Jan Ehrhardt <phpdev ehrhardt.nl>]
diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c
index fb679ad3dec..f8615363706 100644
--- a/modules/http2/h2_conn_io.c
+++ b/modules/http2/h2_conn_io.c
@@ -394,7 +394,7 @@ apr_status_t h2_conn_io_pass(h2_conn_io *io, apr_bucket_brigade *bb)
             }
             else {
                 /* bucket fits in remain, copy to scratch */
-                read_to_scratch(io, b);
+                status = read_to_scratch(io, b);
                 apr_bucket_delete(b);
                 continue;
             }
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index f2698c48b3a..2d4e0a41e77 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -569,6 +569,7 @@ static int on_send_data_cb(nghttp2_session *ngh2,
     int eos;
     h2_stream *stream;
     apr_bucket *b;
+    apr_off_t len = length;
     
     (void)ngh2;
     (void)source;
@@ -577,41 +578,53 @@ static int on_send_data_cb(nghttp2_session *ngh2,
     }
     padlen = (unsigned char)frame->data.padlen;
     
+    ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c,
+                  "h2_stream(%ld-%d): send_data_cb for %ld bytes",
+                  session->id, (int)stream_id, (long)length);
+                  
     stream = get_stream(session, stream_id);
     if (!stream) {
         ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_NOTFOUND, session->c,
                       APLOGNO(02924) 
-                      "h2_stream(%ld-%d): send_data",
+                      "h2_stream(%ld-%d): send_data, lookup stream",
                       session->id, (int)stream_id);
         return NGHTTP2_ERR_CALLBACK_FAILURE;
     }
     
-    ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c,
-                  "h2_stream(%ld-%d): send_data_cb for %ld bytes",
-                  session->id, (int)stream_id, (long)length);
-                  
     status = h2_conn_io_write(&session->io, (const char *)framehd, 9);
     if (padlen && status == APR_SUCCESS) {
         status = h2_conn_io_write(&session->io, (const char *)&padlen, 1);
     }
     
-    if (status == APR_SUCCESS) {
-        apr_off_t len = length;
-        status = h2_stream_read_to(stream, session->bbtmp, &len, &eos);
-        if (status == APR_SUCCESS && len != length) {
-            status = APR_EINVAL;
-        }
+    if (status != APR_SUCCESS) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c,
+                      "h2_stream(%ld-%d): writing frame header",
+                      session->id, (int)stream_id);
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
     }
     
-    if (status == APR_SUCCESS && padlen) {
+    status = h2_stream_read_to(stream, session->bbtmp, &len, &eos);
+    if (status != APR_SUCCESS) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c,
+                      "h2_stream(%ld-%d): send_data_cb, reading stream",
+                      session->id, (int)stream_id);
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
+    }
+    else if (len != length) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c,
+                      "h2_stream(%ld-%d): send_data_cb, wanted %ld bytes, "
+                      "got %ld from stream",
+                      session->id, (int)stream_id, (long)length, (long)len);
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
+    }
+    
+    if (padlen) {
         b = apr_bucket_immortal_create(immortal_zeros, padlen, 
                                        session->c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(session->bbtmp, b);
     }
     
-    if (status == APR_SUCCESS) {
-        status = h2_conn_io_pass(&session->io, session->bbtmp);
-    }
+    status = h2_conn_io_pass(&session->io, session->bbtmp);
         
     apr_brigade_cleanup(session->bbtmp);
     if (status == APR_SUCCESS) {
@@ -623,9 +636,8 @@ static int on_send_data_cb(nghttp2_session *ngh2,
                       APLOGNO(02925) 
                       "h2_stream(%ld-%d): failed send_data_cb",
                       session->id, (int)stream_id);
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
     }
-    
-    return h2_session_status_from_apr_status(status);
 }
 
 static int on_frame_send_cb(nghttp2_session *ngh2, 
diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h
index 2b9ef754e6e..abf69c1d824 100644
--- a/modules/http2/h2_version.h
+++ b/modules/http2/h2_version.h
@@ -26,7 +26,7 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.5.9"
+#define MOD_HTTP2_VERSION "1.5.11"
 
 /**
  * @macro
@@ -34,7 +34,7 @@
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010509
+#define MOD_HTTP2_VERSION_NUM 0x01050b
 
 
 #endif /* mod_h2_h2_version_h */