From: Lukáš Ježek Date: Tue, 19 Jan 2021 14:37:13 +0000 (+0100) Subject: doh2: fix sending errors X-Git-Tag: v5.3.1~13^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aa7373a6735ab066719f1533cdff6ccf145ee697;p=thirdparty%2Fknot-resolver.git doh2: fix sending errors --- diff --git a/daemon/http.c b/daemon/http.c index fd4ad262c..802d96ba7 100644 --- a/daemon/http.c +++ b/daemon/http.c @@ -95,6 +95,9 @@ static int send_padding(struct http_ctx *ctx, uint8_t padlen) return 0; } +static int http_status_remove(struct http_ctx *ctx, struct http_stream_status * stat); +static struct http_stream_status * http_status_get(struct http_ctx *ctx, int32_t stream_id); + /* * Write entire DATA frame to underlying transport layer. * @@ -128,6 +131,8 @@ static int send_data_callback(nghttp2_session *h2, nghttp2_frame *frame, const u if (ret < 0) return NGHTTP2_ERR_CALLBACK_FAILURE; + http_status_remove(ctx, http_status_get(ctx, frame->hd.stream_id)); + return 0; } @@ -147,6 +152,11 @@ static ssize_t read_callback(nghttp2_session *h2, int32_t stream_id, uint8_t *bu size_t avail; size_t send; + if (!source->ptr) { + *data_flags |= NGHTTP2_DATA_FLAG_EOF; + return 0; + } + data = (struct http_data*)source->ptr; avail = data->len - data->pos; send = MIN(avail, length); @@ -166,8 +176,7 @@ static struct http_stream_status * http_status_get(struct http_ctx *ctx, int32_t assert(ctx); struct http_stream_status *stat = NULL; - if (stream_id == ctx->incomplete_stream) - //return ctx->current_stream_index; + if (ctx->current_stream && ctx->current_stream->stream_id == stream_id) return ctx->current_stream; @@ -187,13 +196,18 @@ static int http_status_remove(struct http_ctx *ctx, struct http_stream_status * if (!stat) return 0; - //assert(array_del(ctx->stream_status, idx) == 0); - // TODO - ctx->stream_status.len -= 1; - if (stat->err_msg) - free(stat->err_msg); - stat = ctx->stream_status.at[ctx->stream_status.len]; - //free(stat); + free(stat->err_msg); + stat->err_msg = NULL; + + size_t idx; + for (idx = 0; idx < ctx->stream_status.len; ++idx) { + if (stat->stream_id == ctx->stream_status.at[idx]->stream_id) { + array_del(ctx->stream_status, idx); + free(stat); + break; + } + } + return 0; } @@ -205,17 +219,13 @@ static int send_err_status(struct http_ctx *ctx, int32_t stream_id) int ret; int status_len; nghttp2_data_provider prov; - struct http_stream_status *stat = NULL; + struct http_stream_status *stat = http_status_get(ctx, stream_id); - stat = http_status_get(ctx, stream_id); - assert(stat); - if (stat->err_status == 200) { - http_status_remove(ctx, stat); - return 0; - } + if(!stat) + return kr_error(EINVAL); prov.source.ptr = NULL; - prov.read_callback = NULL; + prov.read_callback = read_callback; char status_str[MAX_DECIMAL_LENGTH(stat->err_status)] = { 0 }; status_len = snprintf(status_str, MAX_DECIMAL_LENGTH(stat->err_status), "%u", stat->err_status); @@ -234,24 +244,23 @@ static int send_err_status(struct http_ctx *ctx, int32_t stream_id) data->on_write = NULL; data->req = NULL; data->ttl = 0; - prov.source.ptr = data; - prov.read_callback = read_callback; } ret = nghttp2_submit_response(ctx->h2, stream_id, hdrs_err, sizeof(hdrs_err)/sizeof(*hdrs_err), &prov); if (ret != 0) return kr_error(EIO); - http_status_remove(ctx, stat); + if (queue_len(ctx->streams) != 0) + queue_pop(ctx->streams); return 0; } /* - * Set error status for particural stream_id and return array index or error + * Set error status for particular stream_id and return stream status or NULL on fail. * - * status_msg is optional and define error message. + * status_msg is optional and defines error message. */ static struct http_stream_status * set_error_status(struct http_ctx *ctx, int32_t stream_id, int status, const char *const status_msg) { @@ -266,6 +275,7 @@ static struct http_stream_status * set_error_status(struct http_ctx *ctx, int32_ if (!stat) return NULL; + // push to end of array if (array_push(ctx->stream_status, stat) < 0) { free(stat); return NULL; @@ -285,13 +295,14 @@ static struct http_stream_status * set_error_status(struct http_ctx *ctx, int32_ return stat; } - stat->err_msg = realloc(stat->err_msg, sizeof(*stat->err_msg) * (strlen(status_msg) + 1)); + stat->err_msg = realloc(stat->err_msg, sizeof(*stat->err_msg) * (strlen(status_msg) + 2)); if (!stat->err_msg) { return stat; } memcpy(stat->err_msg, status_msg, strlen(status_msg)); - stat->err_msg[strlen(status_msg)] = '\0'; + stat->err_msg[strlen(status_msg)] = '\n'; + stat->err_msg[strlen(status_msg)+1] = '\0'; return stat; } @@ -299,17 +310,30 @@ static struct http_stream_status * set_error_status(struct http_ctx *ctx, int32_ /* * Reinit temporaly data of current stream */ -static void http_status_reinit(struct http_ctx *ctx) +static void http_status_reinit(struct http_ctx *ctx, int stream_id) { - ctx->incomplete_stream = -1; ctx->current_method = HTTP_METHOD_NONE; ctx->current_stream = NULL; + ctx->buf_pos = 0; + if (ctx->uri_path) { + free(ctx->uri_path); + ctx->uri_path = NULL; + } if (ctx->content_type) { free(ctx->content_type); ctx->content_type = NULL; } } +static void http_status_reinit_error(struct http_ctx *ctx, int stream_id) +{ + + if (ctx->current_method == HTTP_METHOD_POST) + queue_pop(ctx->streams); + + http_status_reinit(ctx, stream_id); +} + /* * Check endpoint and uri path */ @@ -372,6 +396,11 @@ static int check_uri(struct http_ctx *ctx, int32_t stream_id, const char* uri_pa } } + if (!beg) { /* no dns variable in path */ + stat = set_error_status(ctx, stream_id, 400, "'dns' key in path not found"); + return stat ? kr_error(EINVAL) : kr_error(ENOMEM); + } + } else { if (!beg) { /* no dns variable in path */ stat = set_error_status(ctx, stream_id, 400, "'dns' key in path not found"); return stat ? kr_error(EINVAL) : kr_error(ENOMEM); @@ -400,8 +429,10 @@ static int process_uri_path(struct http_ctx *ctx, int32_t stream_id) } beg = strstr(ctx->uri_path, key); - if (!beg) /* No dns variable in ctx->uri_path. */ - return 0; + if (!beg) { /* No dns variable in ctx->uri_path. */ + stat = set_error_status(ctx, stream_id, 400, "'dns' key in path not found"); + return stat ? 0 : kr_error(ENOMEM); + } beg += sizeof(key) - 1; end = strchr(beg, '&'); @@ -447,13 +478,12 @@ static int begin_headers_callback(nghttp2_session *h2, const nghttp2_frame *fram return 0; } - if (ctx->incomplete_stream != -1) { + if (ctx->current_stream != NULL) { kr_log_verbose( - "[http] stream %d incomplete\n", ctx->incomplete_stream); + "[http] stream %d incomplete\n", ctx->current_stream->stream_id); if (!set_error_status(ctx, stream_id, 501, "incomplete stream")) return NGHTTP2_ERR_CALLBACK_FAILURE; } else { - ctx->incomplete_stream = stream_id; ctx->current_stream = set_error_status(ctx, stream_id, 200, NULL); } return 0; @@ -475,9 +505,9 @@ static int header_callback(nghttp2_session *h2, const nghttp2_frame *frame, if (frame->hd.type != NGHTTP2_HEADERS) return 0; - if (ctx->incomplete_stream != stream_id) { + if (ctx->current_stream->stream_id != stream_id) { kr_log_verbose( - "[http] stream %d incomplete\n", ctx->incomplete_stream); + "[http] stream %d incomplete\n", ctx->current_stream->stream_id); if (!set_error_status(ctx, stream_id, 501, "incomplete stream")) return NGHTTP2_ERR_CALLBACK_FAILURE; return 0; @@ -532,12 +562,13 @@ static int data_chunk_recv_callback(nghttp2_session *h2, uint8_t flags, int32_t struct http_ctx *ctx = (struct http_ctx *)user_data; ssize_t remaining; ssize_t required; - bool is_first = queue_len(ctx->streams) == 0 || queue_tail(ctx->streams) != ctx->incomplete_stream; + assert(ctx->current_stream); + bool is_first = queue_len(ctx->streams) == 0 || queue_tail(ctx->streams) != ctx->current_stream->stream_id; - if (ctx->incomplete_stream != stream_id) { + if (ctx->current_stream->stream_id != stream_id) { kr_log_verbose( "[http] stream %d incomplete\n", - ctx->incomplete_stream); + ctx->current_stream->stream_id); if (!set_error_status(ctx, stream_id, 501, "incomplete stream")) return NGHTTP2_ERR_CALLBACK_FAILURE; return 0; @@ -552,7 +583,7 @@ static int data_chunk_recv_callback(nghttp2_session *h2, uint8_t flags, int32_t if (required > remaining) { kr_log_error("[http] insufficient space in buffer\n"); if (!set_error_status(ctx, stream_id, 413, NULL)) { - http_status_reinit(ctx); + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; } return 0; @@ -583,13 +614,13 @@ static int on_frame_recv_callback(nghttp2_session *h2, const nghttp2_frame *fram int32_t stream_id = frame->hd.stream_id; assert(stream_id != -1); - if (stream_id == 0) + if (stream_id == 0 || ctx == NULL) return 0; if (ctx->current_method == HTTP_METHOD_NONE) { kr_log_verbose("[http] unsupported HTTP method\n"); if (!set_error_status(ctx, stream_id, 405, "only HTTP POST and GET are supported\n")) { - http_status_reinit(ctx); + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; } } @@ -597,55 +628,59 @@ static int on_frame_recv_callback(nghttp2_session *h2, const nghttp2_frame *fram if (ctx->content_type && strcasecmp("application/dns-message", (const char *)ctx->content_type)) { kr_log_verbose("[http] unsupported content-type %s\n", ctx->content_type); if (!set_error_status(ctx, stream_id, 415, "only Content-Type: application/dns-message is supported\n")) { - http_status_reinit(ctx); + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; } } - if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) { + if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { struct http_stream_status *stat = ctx->current_stream; - if (ctx->incomplete_stream == stream_id) { - if (ctx->current_method == HTTP_METHOD_GET) { - if (process_uri_path(ctx, stream_id) < 0) { - http_status_reinit(ctx); - return NGHTTP2_ERR_CALLBACK_FAILURE; - } - } - free(ctx->uri_path); - ctx->uri_path = NULL; - - if (ctx->buf_pos) { - len = ctx->buf_pos - sizeof(uint16_t); - if (len <= 0 || len > KNOT_WIRE_MAX_PKTSIZE) { - kr_log_verbose("[http] invalid dnsmsg size: %zd B\n", len); - http_status_reinit(ctx); - return NGHTTP2_ERR_CALLBACK_FAILURE; + assert(stat); + if (stat->stream_id == stream_id) { + if (stat->err_status == 200) { + if (ctx->current_method == HTTP_METHOD_GET) { + if (process_uri_path(ctx, stream_id) < 0) { + http_status_reinit_error(ctx, stream_id); + return NGHTTP2_ERR_CALLBACK_FAILURE; + } } - if (len < 12) { - if (!set_error_status(ctx, stream_id, 400, "input too short\n")) { - http_status_reinit(ctx); + if (ctx->buf_pos) { + len = ctx->buf_pos - sizeof(uint16_t); + if (len <= 0 || len > KNOT_WIRE_MAX_PKTSIZE) { + kr_log_verbose("[http] invalid dnsmsg size: %zd B\n", len); + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; } - } - if (stat->err_status == 200) { - knot_wire_write_u16(ctx->buf, len); - ctx->submitted += ctx->buf_pos; - ctx->buf += ctx->buf_pos; + if (len < 12) { + if (!set_error_status(ctx, stream_id, 400, "input too short\n")) { + http_status_reinit_error(ctx, stream_id); + return NGHTTP2_ERR_CALLBACK_FAILURE; + } + } + + if (stat->err_status == 200) { + knot_wire_write_u16(ctx->buf, len); + ctx->submitted += ctx->buf_pos; + ctx->buf += ctx->buf_pos; + } } } if (stat->err_status != 200) { - if (send_err_status(ctx, stream_id) < 0) + if (send_err_status(ctx, stream_id) < 0) { + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; + } } - http_status_reinit(ctx); + http_status_reinit(ctx, stream_id); ctx->buf_pos = 0; } else { /* send error for non-processed stream */ if (send_err_status(ctx, stream_id) < 0) { + http_status_reinit_error(ctx, stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; } } @@ -718,7 +753,6 @@ struct http_ctx* http_new(struct session *session, http_send_callback send_cb) ctx->send_cb = send_cb; ctx->session = session; queue_init(ctx->streams); - ctx->incomplete_stream = -1; ctx->current_stream = NULL; ctx->submitted = 0; ctx->current_method = HTTP_METHOD_NONE; @@ -886,9 +920,9 @@ void http_free(struct http_ctx *ctx) if (!ctx) return; - // TODO -// while(ctx->stream_status.len) -// http_status_remove(ctx, 0); + while(ctx->stream_status.len) + http_status_remove(ctx, ctx->stream_status.at[0]); + array_clear(ctx->stream_status); queue_deinit(ctx->streams); nghttp2_session_del(ctx->h2);