]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
http2: fix crash in handling stream weights 11384/head
authorStefan Eissing <stefan@eissing.org>
Mon, 26 Jun 2023 07:03:47 +0000 (09:03 +0200)
committerJay Satiro <raysatiro@yahoo.com>
Wed, 28 Jun 2023 20:32:16 +0000 (16:32 -0400)
- Delay the priority handling until the stream has been opened.

- Add test2404 to reproduce and verify.

Weights may change "on the run", which is why there are checks in
general egress handling. These must not trigger when the stream has not
been opened yet.

Reported-by: jbgoog@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/11379
Closes https://github.com/curl/curl/pull/11384

lib/http2.c
tests/data/Makefile.inc
tests/data/test2404 [new file with mode: 0644]
tests/libtest/Makefile.inc
tests/libtest/lib2404.c [new file with mode: 0644]

index 52ae8ce3df29624ad4d0d3061b203ec62e350785..c6275459a96c91dc63c14ce8c9203e122f0379bc 100644 (file)
@@ -1686,9 +1686,10 @@ static CURLcode h2_progress_egress(struct Curl_cfilter *cf,
   struct stream_ctx *stream = H2_STREAM_CTX(data);
   int rv = 0;
 
-  if((sweight_wanted(data) != sweight_in_effect(data)) ||
-     (data->set.priority.exclusive != data->state.priority.exclusive) ||
-     (data->set.priority.parent != data->state.priority.parent) ) {
+  if(stream && stream->id > 0 &&
+     ((sweight_wanted(data) != sweight_in_effect(data)) ||
+      (data->set.priority.exclusive != data->state.priority.exclusive) ||
+      (data->set.priority.parent != data->state.priority.parent)) ) {
     /* send new weight and/or dependency */
     nghttp2_priority_spec pri_spec;
 
index 2b003d950bb93e96400363c265b379aa18e3147f..ca5faf9b58e364ba1229ec6c9ea5691b0a2d21b6 100644 (file)
@@ -247,7 +247,7 @@ test2200 test2201 test2202 test2203 test2204 test2205 \
 \
 test2300 test2301 test2302 test2303 test2304 test2305 test2306 \
 \
-test2400 test2401 test2402 test2403 \
+test2400 test2401 test2402 test2403 test2404 \
 \
 test2500 test2501 test2502 test2503 \
 \
diff --git a/tests/data/test2404 b/tests/data/test2404
new file mode 100644 (file)
index 0000000..0e22bfe
--- /dev/null
@@ -0,0 +1,109 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP/2
+multi
+verbose logs
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data1 crlf="yes">
+HTTP/1.1 200 OK\r
+Date: Tue, 09 Nov 2010 14:49:00 GMT\r
+Server: server.example.com\r
+Content-Length: 47\r
+\r
+file contents should appear once for each file
+</data1>
+<data2>
+HTTP/1.1 200 OK\r
+Date: Tue, 09 Nov 2010 14:49:00 GMT\r
+Server: server.example.com\r
+Content-Length: 47\r
+\r
+file contents should appear once for each file
+</data2>
+<data3>
+HTTP/1.1 200 OK\r
+Date: Tue, 09 Nov 2010 14:49:00 GMT\r
+Server: server.example.com\r
+Content-Length: 47\r
+\r
+file contents should appear once for each file
+</data3>
+<data4>
+HTTP/1.1 200 OK\r
+Date: Tue, 09 Nov 2010 14:49:00 GMT\r
+Server: server.example.com\r
+Content-Length: 47\r
+\r
+file contents should appear once for each file
+</data4>
+</reply>
+
+# Client-side
+<client>
+<features>
+h2c
+SSL
+</features>
+<server>
+http
+http/2
+</server>
+<tool>
+lib%TESTNUMBER
+</tool>
+ <name>
+HTTP/2 using STREAM_WEIGHTs
+ </name>
+ <command>
+https://%HOSTIP:%HTTP2TLSPORT/path/%TESTNUMBER %HOSTIP %HTTP2TLSPORT
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="yes">
+GET /path/%TESTNUMBER0001 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+X-Forwarded-Proto: https
+Via: 2 nghttpx
+
+GET /path/%TESTNUMBER0002 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+X-Forwarded-Proto: https
+Via: 2 nghttpx
+
+GET /path/%TESTNUMBER0003 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+X-Forwarded-Proto: https
+Via: 2 nghttpx
+
+GET /path/%TESTNUMBER0004 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+X-Forwarded-Proto: https
+Via: 2 nghttpx
+
+</protocol>
+<strip>
+^Host:.*
+</strip>
+<file name="%LOGDIR/stderr%TESTNUMBER" mode="text">
+* Connection #0 to host localhost left intact
+* Connection #0 to host localhost left intact
+* Connection #0 to host localhost left intact
+* Connection #0 to host localhost left intact
+</file>
+<stripfile>
+$_ = '' if (($_ !~ /left intact/) && ($_ !~ /Closing connection/))
+</stripfile>
+</verify>
+</testcase>
index c3b2a9957cc585be99dfb3f331972d6d71f1e8e4..648536eb0a170e13ba34b525f4957dd95150afe6 100644 (file)
@@ -72,7 +72,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect libprereq      \
  lib1960 \
  lib1970 lib1971 lib1972 lib1973 lib1974 lib1975 \
  lib2301 lib2302 lib2304 lib2305 lib2306 \
- lib2402 \
+ lib2402 lib2404 \
  lib2502 \
  lib3010 lib3025 lib3026 lib3027 \
  lib3100 lib3101
@@ -660,6 +660,9 @@ lib2306_LDADD = $(TESTUTIL_LIBS)
 lib2402_SOURCES = lib2402.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
 lib2402_LDADD = $(TESTUTIL_LIBS)
 
+lib2404_SOURCES = lib2404.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
+lib2404_LDADD = $(TESTUTIL_LIBS)
+
 lib2502_SOURCES = lib2502.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
 lib2502_LDADD = $(TESTUTIL_LIBS)
 
diff --git a/tests/libtest/lib2404.c b/tests/libtest/lib2404.c
new file mode 100644 (file)
index 0000000..1a282ff
--- /dev/null
@@ -0,0 +1,144 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Linus Nielsen Feltzing <linus@haxx.se>
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "test.h"
+
+#include "testutil.h"
+#include "warnless.h"
+#include "memdebug.h"
+
+#define TEST_HANG_TIMEOUT 60 * 1000
+
+#define NUM_HANDLES 4
+
+int test(char *URL)
+{
+  int res = 0;
+  CURL *curl[NUM_HANDLES] = {0};
+  int running;
+  CURLM *m = NULL;
+  int i;
+  char target_url[256];
+  char dnsentry[256];
+  struct curl_slist *slist = NULL;
+  char *port = libtest_arg3;
+  char *address = libtest_arg2;
+
+  (void)URL;
+
+  msnprintf(dnsentry, sizeof(dnsentry), "localhost:%s:%s",
+            port, address);
+  printf("%s\n", dnsentry);
+  slist = curl_slist_append(slist, dnsentry);
+  if(!slist) {
+    fprintf(stderr, "curl_slist_append() failed\n");
+    goto test_cleanup;
+  }
+
+  start_test_timing();
+
+  global_init(CURL_GLOBAL_ALL);
+
+  multi_init(m);
+
+  multi_setopt(m, CURLMOPT_MAXCONNECTS, 1L);
+
+  /* get NUM_HANDLES easy handles */
+  for(i = 0; i < NUM_HANDLES; i++) {
+    /* get an easy handle */
+    easy_init(curl[i]);
+    /* specify target */
+    msnprintf(target_url, sizeof(target_url),
+              "https://localhost:%s/path/2404%04i",
+              port, i + 1);
+    target_url[sizeof(target_url) - 1] = '\0';
+    easy_setopt(curl[i], CURLOPT_URL, target_url);
+    /* go http2 */
+    easy_setopt(curl[i], CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
+    /* no peer verify */
+    easy_setopt(curl[i], CURLOPT_SSL_VERIFYPEER, 0L);
+    easy_setopt(curl[i], CURLOPT_SSL_VERIFYHOST, 0L);
+    /* wait for first connection established to see if we can share it */
+    easy_setopt(curl[i], CURLOPT_PIPEWAIT, 1L);
+    /* go verbose */
+    easy_setopt(curl[i], CURLOPT_VERBOSE, 1L);
+    /* include headers */
+    easy_setopt(curl[i], CURLOPT_HEADER, 1L);
+
+    easy_setopt(curl[i], CURLOPT_RESOLVE, slist);
+
+    easy_setopt(curl[i], CURLOPT_STREAM_WEIGHT, (long)128 + i);
+  }
+
+  fprintf(stderr, "Start at URL 0\n");
+
+  for(i = 0; i < NUM_HANDLES; i++) {
+    /* add handle to multi */
+    multi_add_handle(m, curl[i]);
+
+    for(;;) {
+      struct timeval interval;
+      fd_set rd, wr, exc;
+      int maxfd = -99;
+
+      interval.tv_sec = 1;
+      interval.tv_usec = 0;
+
+      multi_perform(m, &running);
+
+      abort_on_test_timeout();
+
+      if(!running)
+        break; /* done */
+
+      FD_ZERO(&rd);
+      FD_ZERO(&wr);
+      FD_ZERO(&exc);
+
+      multi_fdset(m, &rd, &wr, &exc, &maxfd);
+
+      /* At this point, maxfd is guaranteed to be greater or equal than -1. */
+
+      select_test(maxfd + 1, &rd, &wr, &exc, &interval);
+
+      abort_on_test_timeout();
+    }
+    wait_ms(1); /* to ensure different end times */
+  }
+
+test_cleanup:
+
+  /* proper cleanup sequence - type PB */
+
+  for(i = 0; i < NUM_HANDLES; i++) {
+    curl_multi_remove_handle(m, curl[i]);
+    curl_easy_cleanup(curl[i]);
+  }
+
+  curl_slist_free_all(slist);
+
+  curl_multi_cleanup(m);
+  curl_global_cleanup();
+
+  return res;
+}