- Final round of DoH review feedback processing.

diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c
index 27f92a9dc73d4d2276f329fc218c7ec6fa45710d..7833497b1dfe5c6e4ed0e53b3d00b274be0ccfec 100644 (file)
--- a/services/listen_dnsport.c
+++ b/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 ||

diff --git a/services/mesh.c b/services/mesh.c
index c2afdbf820d5099e06f668cc0ff540306363ca4a..ffb6d092ac0c327e6294a906358017f9f23e3fd9 100644 (file)
--- a/services/mesh.c
+++ b/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

diff --git a/testcode/dohclient.c b/testcode/dohclient.c
index 060d71ee4697b364ed66b7477bc6a3cd9c964dac..adcc7d831554595122cbf4be69f2479fbd822a22 100644 (file)
--- a/testcode/dohclient.c
+++ b/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 */

diff --git a/testdata/doh_downstream.tdir/doh_downstream.test b/testdata/doh_downstream.tdir/doh_downstream.test
index 98a54af26c4cb37533c24a07e7c6e34cc05f8980..78e2e84eb3c4b44ff5436548c0d5847c7087febc 100644 (file)
--- a/testdata/doh_downstream.tdir/doh_downstream.test
+++ b/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

diff --git a/testdata/doh_downstream_endpoint.tdir/doh_downstream_endpoint.test b/testdata/doh_downstream_endpoint.tdir/doh_downstream_endpoint.test
index 0e25a47f67752a9dbee6732276b4c16ff9a84a34..d788e366700c82191877870fd1540dd05655f246 100644 (file)
--- a/testdata/doh_downstream_endpoint.tdir/doh_downstream_endpoint.test
+++ b/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

diff --git a/testdata/doh_downstream_post.tdir/doh_downstream_post.test b/testdata/doh_downstream_post.tdir/doh_downstream_post.test
index 4a49c8b5187af3c1596bb2a70182a4cb6557b6b6..d6a512ae324e8494a7cda830242eb1df91f2ee31 100644 (file)
--- a/testdata/doh_downstream_post.tdir/doh_downstream_post.test
+++ b/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

diff --git a/util/configparser.y b/util/configparser.y
index 6ef42628c681758b5a608c6781b080d466b97ccb..ffc8813ff99a74d8136bdade32e21fc71a63cf2a 100644 (file)
--- a/util/configparser.y
+++ b/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 {

diff --git a/util/netevent.c b/util/netevent.c
index be3b740b592bfb7af78d1d9dbf951f855a2b3068..2171e3e6d068b1a8b4e8177d7e6e74db84d754f2 100644 (file)
--- a/util/netevent.c
+++ b/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;

diff --git a/util/netevent.h b/util/netevent.h
index 68f4c1297a751e1a873fe37037dc38ce350093bb..6986f881b38a7c3eb80ce1a53bd77c9db575008c 100644 (file)
--- a/util/netevent.h
+++ b/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 */