Properly close stream on HTTP2 failure

Author: danielinux

src/Makefile

@@ -22,7 +22,7 @@ asan: CFLAGS+=-fsanitize=address
 asan: LDFLAGS+=-fsanitize=address
 asan: dohd
 
-dohd: url64.o libevquick.o mempool.o odoh.o proxy_auth.o dohd.o
+dohd: url64.o libevquick.o mempool.o odoh.o proxy_auth.o h2_session.o dohd.o
 	gcc -o $@ $^ $(LDFLAGS)
 clean:
 	rm -f *.o dohd

src/dohd.c

@@ -37,6 +37,7 @@
 #include "mempool.h"
 #include "odoh.h"
 #include "proxy_auth.h"
+#include "h2_session.h"
 #include <nghttp2/nghttp2.h>
 #include <netinet/tcp.h>
 #include <fcntl.h>
@@ -1018,13 +1019,7 @@ static ssize_t h2_cb_send(nghttp2_session *session, const uint8_t *data,
     struct client_data *cd = (struct client_data *)user_data;
     (void)session;
     (void)flags;
-    int ret;
-    ret = client_ssl_write(cd, data, length);
-
-    if (nghttp2_session_want_write(session)) {
-        nghttp2_session_send(session);
-    }
-    return ret;
+    return client_ssl_write(cd, data, length);
 }
 
 
@@ -1111,12 +1106,9 @@ static int h2_cb_on_stream_close(nghttp2_session *session, int32_t stream_id,
     struct client_data *cd = (struct client_data *)user_data;
     struct req_slot *req =
         nghttp2_session_get_stream_user_data(session, stream_id);
-    if (!req)
-        return -1;
-    if (cd != req->owner)
-        return -1;
     (void)error_code;
-    if (req)  {
+    if (dohd_h2_stream_close_action(req != NULL,
+            req != NULL && cd == req->owner) == DOHD_H2_STREAM_CLOSE_DESTROY) {
         dohd_destroy_request(req);
     }
     return 0;
@@ -1281,12 +1273,15 @@ static void tls_read(__attribute__((unused)) int fd, short __attribute__((unused
             readlen = nghttp2_session_mem_recv(cd->h2_session, buff, ret);
             if (readlen < 0) {
                 dohprint(DOH_WARN, "NGHTTP2 error: %s\n", nghttp2_strerror((int)readlen));
+                dohd_destroy_client(cd);
                 return;
             }
             while (nghttp2_session_want_write(cd->h2_session)) {
                 ret = nghttp2_session_send(cd->h2_session);
                 if (ret < 0) {
                     dohprint(DOH_WARN, "NGHTTP2 error: %s\n", nghttp2_strerror((int)ret));
+                    dohd_destroy_client(cd);
+                    return;
                 }
             }
         } else {

test/Makefile

@@ -8,7 +8,7 @@ CFLAGS := -I../src -O0 -ggdb -Wall -Wextra -Wno-sign-compare \
 CFLAGS += $(if $(shell ldd /bin/ls | grep 'musl' | head -1 | cut -d ' ' -f1), -D_MUSL_,)
 
 # Default target: build all unit tests
-all: dohd_url64_test dohd_url64_extended_test dohd_heap_test dohd_dns_parser_test dohd_mempool_test
+all: dohd_url64_test dohd_url64_extended_test dohd_heap_test dohd_dns_parser_test dohd_mempool_test dohd_h2_session_test
 
 # Run all unit tests
 check: all
@@ -29,6 +29,9 @@ check: all
 	@echo "--- Memory Pool Tests ---"
 	./dohd_mempool_test
 	@echo ""
+	@echo "--- HTTP/2 Session Tests ---"
+	./dohd_h2_session_test
+	@echo ""
 	@echo "=== All Unit Tests Passed ==="
 
 # Original url64 test
@@ -51,6 +54,10 @@ dohd_dns_parser_test: test_dns_parser.o
 dohd_mempool_test: mempool.o test_mempool.o
 	${CC} -o dohd_mempool_test mempool.o test_mempool.o
 
+# HTTP/2 session helper test
+dohd_h2_session_test: h2_session.o test_h2_session.o
+	${CC} -o dohd_h2_session_test h2_session.o test_h2_session.o
+
 # Object files
 url64.o: ../src/url64.c
 	${CC} ${CFLAGS} -c -o url64.o ../src/url64.c
@@ -73,9 +80,15 @@ test_dns_parser.o: test_dns_parser.c
 mempool.o: ../src/mempool.c ../src/mempool.h
 	${CC} ${CFLAGS} -c -o mempool.o ../src/mempool.c
 
+h2_session.o: ../src/h2_session.c ../src/h2_session.h
+	${CC} ${CFLAGS} -c -o h2_session.o ../src/h2_session.c
+
 test_mempool.o: test_mempool.c ../src/mempool.h
 	${CC} ${CFLAGS} -c -o test_mempool.o test_mempool.c
 
+test_h2_session.o: test_h2_session.c ../src/h2_session.h
+	${CC} ${CFLAGS} -c -o test_h2_session.o test_h2_session.c
+
 # Integration tests (require running dohd)
 integration: gen-certs
 	@echo "=== Running Integration Tests ==="
@@ -126,7 +139,7 @@ gen-certs:
 
 clean:
 	rm -f *.o
-	rm -f dohd_url64_test dohd_url64_extended_test dohd_heap_test dohd_dns_parser_test dohd_mempool_test
+	rm -f dohd_url64_test dohd_url64_extended_test dohd_heap_test dohd_dns_parser_test dohd_mempool_test dohd_h2_session_test
 	rm -f test.crt test.key valgrind.log
 
 .PHONY: all check integration valgrind gen-certs clean