http2: fix copynpaste error reported by coverity

- move all code handling HTTP/2 frames for a particular
  stream into a separate function to keep from confusing
  the call `data` with the stream `data`.

Closes #10924

diff --git a/lib/http2.c b/lib/http2.c
index 127ae4394f351da29dc235374c48581088fea932..86b3534853df86e29cf34203db1e3df1ebc8e946 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -950,98 +950,44 @@ static CURLcode recvbuf_write_hds(struct Curl_cfilter *cf,
   return CURLE_OK;
 }
 
-static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
-                         void *userp)
+static CURLcode on_stream_frame(struct Curl_cfilter *cf,
+                                struct Curl_easy *data,
+                                const nghttp2_frame *frame)
 {
-  struct Curl_cfilter *cf = userp;
   struct cf_h2_ctx *ctx = cf->ctx;
-  struct Curl_easy *data_s = NULL;
-  struct stream_ctx *stream = NULL;
-  struct Curl_easy *data = CF_DATA_CURRENT(cf);
-  int rv;
+  struct stream_ctx *stream = H2_STREAM_CTX(data);
   int32_t stream_id = frame->hd.stream_id;
   CURLcode result;
+  int rv;
 
-  DEBUGASSERT(data);
-  if(!stream_id) {
-    /* stream ID zero is for connection-oriented stuff */
-    DEBUGASSERT(data);
-    switch(frame->hd.type) {
-    case NGHTTP2_SETTINGS: {
-      uint32_t max_conn = ctx->max_concurrent_streams;
-      DEBUGF(LOG_CF(data, cf, "recv frame SETTINGS"));
-      ctx->max_concurrent_streams = nghttp2_session_get_remote_settings(
-          session, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
-      ctx->enable_push = nghttp2_session_get_remote_settings(
-          session, NGHTTP2_SETTINGS_ENABLE_PUSH) != 0;
-      DEBUGF(LOG_CF(data, cf, "MAX_CONCURRENT_STREAMS == %d",
-                    ctx->max_concurrent_streams));
-      DEBUGF(LOG_CF(data, cf, "ENABLE_PUSH == %s",
-                    ctx->enable_push ? "TRUE" : "false"));
-      if(data && max_conn != ctx->max_concurrent_streams) {
-        /* only signal change if the value actually changed */
-        DEBUGF(LOG_CF(data, cf, "MAX_CONCURRENT_STREAMS now %u",
-                      ctx->max_concurrent_streams));
-        multi_connchanged(data->multi);
-      }
-      break;
-    }
-    case NGHTTP2_GOAWAY:
-      ctx->goaway = TRUE;
-      ctx->goaway_error = frame->goaway.error_code;
-      ctx->last_stream_id = frame->goaway.last_stream_id;
-      if(data) {
-        DEBUGF(LOG_CF(data, cf, "recv GOAWAY, error=%d, last_stream=%u",
-                      ctx->goaway_error, ctx->last_stream_id));
-        infof(data, "recveived GOAWAY, error=%d, last_stream=%u",
-                    ctx->goaway_error, ctx->last_stream_id);
-        multi_connchanged(data->multi);
-      }
-      break;
-    case NGHTTP2_WINDOW_UPDATE:
-      DEBUGF(LOG_CF(data, cf, "recv frame WINDOW_UPDATE"));
-      break;
-    default:
-      DEBUGF(LOG_CF(data, cf, "recv frame %x on 0", frame->hd.type));
-    }
-    return 0;
-  }
-  data_s = nghttp2_session_get_stream_user_data(session, stream_id);
-  if(!data_s) {
-    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] No Curl_easy associated",
-                  stream_id));
-    return 0;
-  }
-
-  stream = H2_STREAM_CTX(data_s);
   if(!stream) {
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] No proto pointer", stream_id));
-    return NGHTTP2_ERR_CALLBACK_FAILURE;
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] No proto pointer", stream_id));
+    return CURLE_FAILED_INIT;
   }
 
   switch(frame->hd.type) {
   case NGHTTP2_DATA:
     /* If !body started on this stream, then receiving DATA is illegal. */
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] recv frame DATA", stream_id));
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv frame DATA", stream_id));
     if(!stream->bodystarted) {
-      rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
+      rv = nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE,
                                      stream_id, NGHTTP2_PROTOCOL_ERROR);
 
       if(nghttp2_is_fatal(rv)) {
-        return NGHTTP2_ERR_CALLBACK_FAILURE;
+        return CURLE_RECV_ERROR;
       }
     }
     if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
       /* Stream has ended. If there is pending data, ensure that read
          will occur to consume it. */
       if(!data->state.drain && !Curl_bufq_is_empty(&stream->recvbuf)) {
-        drain_this(cf, data_s);
+        drain_this(cf, data);
         Curl_expire(data, 0, EXPIRE_RUN_NOW);
       }
     }
     break;
   case NGHTTP2_HEADERS:
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] recv frame HEADERS", stream_id));
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv frame HEADERS", stream_id));
     if(stream->bodystarted) {
       /* Only valid HEADERS after body started is trailer HEADERS.  We
          buffer them in on_header callback. */
@@ -1052,7 +998,7 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
        stream->status_code. Fuzzing has proven this can still be reached
        without status code having been set. */
     if(stream->status_code == -1)
-      return NGHTTP2_ERR_CALLBACK_FAILURE;
+      return CURLE_RECV_ERROR;
 
     /* Only final status code signals the end of header */
     if(stream->status_code / 100 != 1) {
@@ -1060,36 +1006,36 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
       stream->status_code = -1;
     }
 
-    result = recvbuf_write_hds(cf, data_s, STRCONST("\r\n"));
+    result = recvbuf_write_hds(cf, data, STRCONST("\r\n"));
     if(result)
-      return NGHTTP2_ERR_CALLBACK_FAILURE;
+      return result;
 
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] %zu header bytes",
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] %zu header bytes",
                   stream_id, Curl_bufq_len(&stream->recvbuf)));
-    if(CF_DATA_CURRENT(cf) != data_s) {
-      drain_this(cf, data_s);
-      Curl_expire(data_s, 0, EXPIRE_RUN_NOW);
+    if(CF_DATA_CURRENT(cf) != data) {
+      drain_this(cf, data);
+      Curl_expire(data, 0, EXPIRE_RUN_NOW);
     }
     break;
   case NGHTTP2_PUSH_PROMISE:
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] recv PUSH_PROMISE", stream_id));
-    rv = push_promise(cf, data_s, &frame->push_promise);
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv PUSH_PROMISE", stream_id));
+    rv = push_promise(cf, data, &frame->push_promise);
     if(rv) { /* deny! */
-      int h2;
       DEBUGASSERT((rv > CURL_PUSH_OK) && (rv <= CURL_PUSH_ERROROUT));
-      h2 = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
+      rv = nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE,
                                      frame->push_promise.promised_stream_id,
                                      NGHTTP2_CANCEL);
-      if(nghttp2_is_fatal(h2))
-        return NGHTTP2_ERR_CALLBACK_FAILURE;
+      if(nghttp2_is_fatal(rv))
+        return CURLE_SEND_ERROR;
       else if(rv == CURL_PUSH_ERROROUT) {
-        DEBUGF(LOG_CF(data_s, cf, "Fail the parent stream (too)"));
-        return NGHTTP2_ERR_CALLBACK_FAILURE;
+        DEBUGF(LOG_CF(data, cf, "[h2sid=%d] fail in PUSH_PROMISE received",
+                      stream_id));
+        return CURLE_RECV_ERROR;
       }
     }
     break;
   case NGHTTP2_RST_STREAM:
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] recv RST", stream_id));
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv RST", stream_id));
     stream->closed = TRUE;
     stream->reset = TRUE;
     drain_this(cf, data);
@@ -1097,21 +1043,84 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
     break;
   case NGHTTP2_WINDOW_UPDATE:
     DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv WINDOW_UPDATE", stream_id));
-    if((data_s->req.keepon & KEEP_SEND_HOLD) &&
-       (data_s->req.keepon & KEEP_SEND)) {
-      data_s->req.keepon &= ~KEEP_SEND_HOLD;
-      drain_this(cf, data_s);
-      Curl_expire(data_s, 0, EXPIRE_RUN_NOW);
+    if((data->req.keepon & KEEP_SEND_HOLD) &&
+       (data->req.keepon & KEEP_SEND)) {
+      data->req.keepon &= ~KEEP_SEND_HOLD;
+      drain_this(cf, data);
+      Curl_expire(data, 0, EXPIRE_RUN_NOW);
       DEBUGF(LOG_CF(data, cf, "[h2sid=%d] un-holding after win update",
                     stream_id));
     }
     break;
   default:
-    DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] recv frame %x",
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] recv frame %x",
                   stream_id, frame->hd.type));
     break;
   }
-  return 0;
+  return CURLE_OK;
+}
+
+static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
+                         void *userp)
+{
+  struct Curl_cfilter *cf = userp;
+  struct cf_h2_ctx *ctx = cf->ctx;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf), *data_s;
+  int32_t stream_id = frame->hd.stream_id;
+
+  DEBUGASSERT(data);
+  if(!stream_id) {
+    /* stream ID zero is for connection-oriented stuff */
+    DEBUGASSERT(data);
+    switch(frame->hd.type) {
+    case NGHTTP2_SETTINGS: {
+      uint32_t max_conn = ctx->max_concurrent_streams;
+      DEBUGF(LOG_CF(data, cf, "recv frame SETTINGS"));
+      ctx->max_concurrent_streams = nghttp2_session_get_remote_settings(
+          session, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
+      ctx->enable_push = nghttp2_session_get_remote_settings(
+          session, NGHTTP2_SETTINGS_ENABLE_PUSH) != 0;
+      DEBUGF(LOG_CF(data, cf, "MAX_CONCURRENT_STREAMS == %d",
+                    ctx->max_concurrent_streams));
+      DEBUGF(LOG_CF(data, cf, "ENABLE_PUSH == %s",
+                    ctx->enable_push ? "TRUE" : "false"));
+      if(data && max_conn != ctx->max_concurrent_streams) {
+        /* only signal change if the value actually changed */
+        DEBUGF(LOG_CF(data, cf, "MAX_CONCURRENT_STREAMS now %u",
+                      ctx->max_concurrent_streams));
+        multi_connchanged(data->multi);
+      }
+      break;
+    }
+    case NGHTTP2_GOAWAY:
+      ctx->goaway = TRUE;
+      ctx->goaway_error = frame->goaway.error_code;
+      ctx->last_stream_id = frame->goaway.last_stream_id;
+      if(data) {
+        DEBUGF(LOG_CF(data, cf, "recv GOAWAY, error=%d, last_stream=%u",
+                      ctx->goaway_error, ctx->last_stream_id));
+        infof(data, "recveived GOAWAY, error=%d, last_stream=%u",
+                    ctx->goaway_error, ctx->last_stream_id);
+        multi_connchanged(data->multi);
+      }
+      break;
+    case NGHTTP2_WINDOW_UPDATE:
+      DEBUGF(LOG_CF(data, cf, "recv frame WINDOW_UPDATE"));
+      break;
+    default:
+      DEBUGF(LOG_CF(data, cf, "recv frame %x on 0", frame->hd.type));
+    }
+    return 0;
+  }
+
+  data_s = nghttp2_session_get_stream_user_data(session, stream_id);
+  if(!data_s) {
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] No Curl_easy associated",
+                  stream_id));
+    return 0;
+  }
+
+  return on_stream_frame(cf, data_s, frame)? NGHTTP2_ERR_CALLBACK_FAILURE : 0;
 }
 
 static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,