- Final round of DoH review feedback processing.

services/listen_dnsport.c

@@ -2227,7 +2227,7 @@ int http2_submit_dns_response(struct http2_session* h2_session)
 
 	sldns_buffer_write(h2_stream->rbuffer,
 		sldns_buffer_current(h2_session->c->buffer),
-		sldns_buffer_remaining(h2_stream->rbuffer));
+		sldns_buffer_remaining(h2_session->c->buffer));
 	sldns_buffer_flip(h2_stream->rbuffer);
 
 	data_prd.source.ptr = h2_session;
@@ -2540,14 +2540,16 @@ static int http2_buffer_uri_query(struct http2_session* h2_session,
 			"in http2-query-buffer-size");
 		return http2_submit_rst_stream(h2_session, h2_stream);
 	}
+	http2_query_buffer_count += expectb64len;
+	lock_basic_unlock(&http2_query_buffer_count_lock);
 	if(!(h2_stream->qbuffer = sldns_buffer_new(expectb64len))) {
+		lock_basic_lock(&http2_query_buffer_count_lock);
+		http2_query_buffer_count -= expectb64len;
 		lock_basic_unlock(&http2_query_buffer_count_lock);
 		log_err("http2_req_header fail, qbuffer "
 			"malloc failure");
 		return 0;
 	}
-	http2_query_buffer_count += expectb64len;
-	lock_basic_unlock(&http2_query_buffer_count_lock);
 
 	if(!(b64len = sldns_b64url_pton(
 		(char const *)start, length,
@@ -2627,7 +2629,7 @@ static int http2_req_header_cb(nghttp2_session* session,
 		 * stream. */
 #define	HTTP_QUERY_PARAM "?dns="
 		size_t el = strlen(h2_session->c->http_endpoint);
-		size_t qpl = sizeof(HTTP_QUERY_PARAM) - 1;
+		size_t qpl = strlen(HTTP_QUERY_PARAM);
 
 		if(valuelen < el || memcmp(h2_session->c->http_endpoint,
 			value, el) != 0) {
@@ -2727,9 +2729,13 @@ static int http2_req_data_chunk_recv_cb(nghttp2_session* ATTR_UNUSED(session),
 				"in http2-query-buffer-size");
 			return http2_submit_rst_stream(h2_session, h2_stream);
 		}
-		if((h2_stream->qbuffer = sldns_buffer_new(qlen)))
-			http2_query_buffer_count += qlen;
+		http2_query_buffer_count += qlen;
 		lock_basic_unlock(&http2_query_buffer_count_lock);
+		if(!(h2_stream->qbuffer = sldns_buffer_new(qlen))) {
+			lock_basic_lock(&http2_query_buffer_count_lock);
+			http2_query_buffer_count -= qlen;
+			lock_basic_unlock(&http2_query_buffer_count_lock);
+		}
 	}
 
 	if(!h2_stream->qbuffer ||

services/mesh.c

@@ -551,7 +551,7 @@ void mesh_new_client(struct mesh_area* mesh, struct query_info* qinfo,
 			goto servfail_mem;
 		}
 	}
-	if(rep->c->alpn_h2) {
+	if(rep->c->use_h2) {
 		http2_stream_add_meshstate(rep->c->h2_stream, mesh, s);
 	}
 	/* add serve expired timer if required and not already there */
@@ -1210,7 +1210,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 	else	secure = 0;
 	if(!rep && rcode == LDNS_RCODE_NOERROR)
 		rcode = LDNS_RCODE_SERVFAIL;
-	if(r->query_reply.c->alpn_h2) {
+	if(r->query_reply.c->use_h2) {
 		r->query_reply.c->h2_stream = r->h2_stream;
 		/* Mesh reply won't exist for long anymore. Make it impossible
 		 * for HTTP/2 stream to refer to mesh state, in case
@@ -1498,7 +1498,7 @@ int mesh_state_add_reply(struct mesh_state* s, struct edns_data* edns,
 		s->s.qinfo.qname_len);
 	if(!r->qname)
 		return 0;
-	if(rep->c->alpn_h2)
+	if(rep->c->use_h2)
 		r->h2_stream = rep->c->h2_stream;
 
 	/* Data related to local alias stored in 'qinfo' (if any) is ephemeral

testcode/dohclient.c

@@ -171,8 +171,9 @@ submit_query(struct http2_session* h2_session, struct sldns_buffer* buf)
 		h2_stream->path = malloc(strlen(
 			h2_session->endpoint)+strlen("?dns=")+qb64_size+1);
 		if(!h2_stream->path) fatal_exit("out of memory");
-		sprintf(h2_stream->path, "%s?dns=%s", h2_session->endpoint,
-			qb64);
+		snprintf(h2_stream->path, strlen(h2_session->endpoint)+
+			strlen("?dns=")+qb64_size+1, "%s?dns=%s",
+			h2_session->endpoint, qb64);
 		free(qb64);
 	}
 
@@ -328,6 +329,11 @@ static int http2_data_chunk_recv_cb(nghttp2_session* ATTR_UNUSED(session),
 		return 0;
 	}
 
+	if(sldns_buffer_remaining(h2_stream->buf) < len) {
+		log_err("received data chunck does not fit into buffer");
+		return NGHTTP2_ERR_CALLBACK_FAILURE;
+	}
+
 	sldns_buffer_write(h2_stream->buf, data, len);
 
 	return 0;
@@ -575,5 +581,6 @@ int main(int argc, char** argv)
 int main(int ATTR_UNUSED(argc), char** ATTR_UNUSED(argv))
 {
 	printf("Compiled without nghttp2, cannot run test.\n");
+	return 1;
 }
 #endif /*  HAVE_NGHTTP2 */

testdata/doh_downstream.tdir/doh_downstream.test

@@ -323,17 +323,12 @@ if test "$?" -ne 0; then
 	echo "Not OK"
 	exit 1
 fi
-grep "a.example.com.	IN	A" outfile
-
-echo ""
-echo "> query www5.example.net. www3.example.net. www.drop.net."
-$PRE/dohclient -s 127.0.0.1 -p $UNBOUND_PORT www5.example.com. A IN www3.example.net A IN www.drop.net A IN >outfile 2>&1
-cat outfile
-if test "$?" -ne 0; then
-	echo "exit status not OK"
+num_ans=$(grep -B 3 "a.example.com.	IN	A" outfile | grep "rcode: NOERROR" | wc -l )
+if test "$num_ans" -ne 90; then
+	echo "number of answers not OK"
 	echo "> cat logfiles"
 	cat outfile
-	cat fwd.log 
+	cat fwd.log
 	cat unbound.log
 	echo "Not OK"
 	exit 1

testdata/doh_downstream_endpoint.tdir/doh_downstream_endpoint.test

@@ -33,7 +33,7 @@ else
 fi
 echo "OK"
 
-echo "> query www.example.net. endpoint /dns-query"
+echo "> query www.example.net. endpoint /abc"
 $PRE/dohclient -e /abc -s 127.0.0.1 -p $UNBOUND_PORT www.example.net. A IN >outfile 2>&1
 cat outfile
 if test "$?" -ne 0; then

testdata/doh_downstream_post.tdir/doh_downstream_post.test

@@ -323,17 +323,13 @@ if test "$?" -ne 0; then
 	echo "Not OK"
 	exit 1
 fi
-grep "a.example.com.	IN	A" outfile
 
-echo ""
-echo "> query www5.example.net. www3.example.net. www.drop.net."
-$PRE/dohclient -P -s 127.0.0.1 -p $UNBOUND_PORT www5.example.com. A IN www3.example.net A IN www.drop.net A IN >outfile 2>&1
-cat outfile
-if test "$?" -ne 0; then
-	echo "exit status not OK"
+num_ans=$(grep -B 3 "a.example.com.	IN	A" outfile | grep "rcode: NOERROR" | wc -l )
+if test "$num_ans" -ne 90; then
+	echo "number of answers not OK"
 	echo "> cat logfiles"
 	cat outfile
-	cat fwd.log 
+	cat fwd.log
 	cat unbound.log
 	echo "Not OK"
 	exit 1

util/configparser.y

@@ -982,8 +982,10 @@ server_http_endpoint: VAR_HTTP_ENDPOINT STRING_ARG
 		free(cfg_parser->cfg->http_endpoint);
 		if($2 && $2[0] != '/') {
 			cfg_parser->cfg->http_endpoint = malloc(strlen($2)+2);
+			if(!cfg_parser->cfg->http_endpoint)
+				yyerror("out of memory");
 			cfg_parser->cfg->http_endpoint[0] = '/';
-			memcpy(cfg_parser->cfg->http_endpoint+1, $2,
+			memmove(cfg_parser->cfg->http_endpoint+1, $2,
 				strlen($2)+1);
 			free($2);
 		} else {

util/netevent.c

@@ -1228,7 +1228,7 @@ ssl_handshake(struct comm_point* c)
 		if(alpnlen == 2 && memcmp("h2", alpn, 2) == 0) {
 			/* connection upgraded to HTTP2 */
 			c->tcp_do_toggle_rw = 0;
-			c->alpn_h2 = 1;
+			c->use_h2 = 1;
 		}
 	}
 
@@ -2472,7 +2472,7 @@ comm_point_http_handle_read(int fd, struct comm_point* c)
 	if(!c->tcp_is_reading)
 		return 1;
 
-	if(c->alpn_h2) {
+	if(c->use_h2) {
 		return comm_point_http2_handle_read(fd, c);
 	}
 
@@ -2766,7 +2766,7 @@ comm_point_http_handle_write(int fd, struct comm_point* c)
 	if(c->tcp_is_reading)
 		return 1;
 
-	if(c->alpn_h2) {
+	if(c->use_h2) {
 		return comm_point_http2_handle_write(fd, c);
 	}
 
@@ -3160,7 +3160,7 @@ comm_point_create_http_handler(struct comm_base *base,
 		free(c);
 		return NULL;
 	}
-	c->alpn_h2 = 0;
+	c->use_h2 = 0;
 #ifdef HAVE_NGHTTP2
 	if(!(c->h2_session = http2_session_create(c))) {
 		log_err("could not create http2 session");
@@ -3676,7 +3676,7 @@ comm_point_send_reply(struct comm_reply *repinfo)
 #endif
 		if(repinfo->c->tcp_req_info) {
 			tcp_req_info_send_reply(repinfo->c->tcp_req_info);
-		} else if(repinfo->c->alpn_h2) {
+		} else if(repinfo->c->use_h2) {
 			if(!http2_submit_dns_response(repinfo->c->h2_session)) {
 				comm_point_drop_reply(repinfo);
 				return;

util/netevent.h

@@ -236,8 +236,8 @@ struct comm_point {
 	/* -------- HTTP/2 ------- */
 	/** http2 session */
 	struct http2_session* h2_session;
-	/** set to 1 if h2 is negatiated using alpn */
-	int alpn_h2;
+	/** set to 1 if h2 is negotiated to be used (using alpn) */
+	int use_h2;
 	/** stream currently being handled */
 	struct http2_stream* h2_stream;
 	/** maximum allowed query buffer size, per stream */