This is an automated email from the ASF dual-hosted git repository. bcall pushed a commit to branch 8.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/8.0.x by this push: new 60e0a8c Fix chunked processing 60e0a8c is described below commit 60e0a8ce23d390b851873e020483d6f75e857158 Author: Susan Hinrichs <shinr...@verizonmedia.com> AuthorDate: Wed Feb 5 19:15:36 2020 +0000 Fix chunked processing --- iocore/net/UnixNetVConnection.cc | 2 +- proxy/http/HttpTransact.cc | 5 +- proxy/http/HttpTunnel.cc | 160 ++++++++++++++++++++++++--------------- proxy/http/HttpTunnel.h | 1 + 4 files changed, 105 insertions(+), 63 deletions(-) diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc index 3d76002..135b0b5 100644 --- a/iocore/net/UnixNetVConnection.cc +++ b/iocore/net/UnixNetVConnection.cc @@ -933,7 +933,7 @@ UnixNetVConnection::load_buffer_and_write(int64_t towrite, MIOBufferAccessor &bu try_to_write = 0; while (niov < NET_MAX_IOV) { - int64_t wavail = towrite - total_written; + int64_t wavail = towrite - total_written - try_to_write; int64_t len = tmp_reader->block_read_avail(); // Check if we have done this block. diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index e78fdd9..c0ddf77 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -6806,9 +6806,8 @@ HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTP // check that the client is HTTP 1.1 and the conf allows chunking or the client // protocol unchunks before returning to the user agent (i.e. is http/2) - if (s->client_info.http_version == HTTPVersion(1, 1) && - (s->txn_conf->chunking_enabled == 1 || - (s->state_machine->plugin_tag && (!strncmp(s->state_machine->plugin_tag, "http/2", 6)))) && + if (s->client_info.http_version == HTTPVersion(1, 1) && s->txn_conf->chunking_enabled == 1 && + s->state_machine->ua_txn->is_chunked_encoding_supported() && // if we're not sending a body, don't set a chunked header regardless of server response !is_response_body_precluded(s->hdr_info.client_response.status_get(), s->method) && // we do not need chunked encoding for internal error messages diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index 84634c3..bcdb695 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -871,10 +871,9 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) consumer_n = (producer_n = INT64_MAX); } - // Do the IO on the consumers first so - // data doesn't disappear out from - // under the tunnel - for (c = p->consumer_list.head; c;) { + // At least set up the consumer readers first so the data + // doesn't disappear out from under the tunnel + for (c = p->consumer_list.head; c; c = c->link.next) { // Create a reader for each consumer. The reader allows // us to implement skip bytes if (c->vc_type == HT_CACHE_WRITE) { @@ -905,45 +904,6 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) ink_assert(c->skip_bytes <= c->buffer_reader->read_avail()); c->buffer_reader->consume(c->skip_bytes); } - int64_t c_write = consumer_n; - - // INKqa05109 - if we don't know the length leave it at - // INT64_MAX or else the cache may bounce the write - // because it thinks the document is too big. INT64_MAX - // is a special case for the max document size code - // in the cache - if (c_write != INT64_MAX) { - c_write -= c->skip_bytes; - } - // Fix for problems with not chunked content being chunked and - // not sending the entire data. The content length grows when - // it is being chunked. - if (p->do_chunking == true) { - c_write = INT64_MAX; - } - - if (c_write == 0) { - // Nothing to do, call back the cleanup handlers - c->write_vio = nullptr; - consumer_handler(VC_EVENT_WRITE_COMPLETE, c); - } else { - // In the client half close case, all the data that will be sent - // from the client is already in the buffer. Go ahead and set - // the amount to read since we know it. We will forward the FIN - // to the server on VC_EVENT_WRITE_COMPLETE. - if (p->vc_type == HT_HTTP_CLIENT) { - ProxyClientTransaction *ua_vc = static_cast<ProxyClientTransaction *>(p->vc); - if (ua_vc->get_half_close_flag()) { - c_write = c->buffer_reader->read_avail(); - p->alive = false; - p->handler_state = HTTP_SM_POST_SUCCESS; - } - } - c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader); - ink_assert(c_write > 0); - } - - c = c->link.next; } // YTS Team, yamsat Plugin @@ -1003,7 +963,56 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) producer_n = 0; } } + for (c = p->consumer_list.head; c; c = c->link.next) { + int64_t c_write = consumer_n; + + if (!p->alive) { + // Adjust the amount of chunked data to write if the only data was in the initial read + // The amount to write in some cases is dependent on the type of the consumer, so this + // value must be computed for each consumer + c_write = final_consumer_bytes_to_write(p, c); + } else { + // INKqa05109 - if we don't know the length leave it at + // INT64_MAX or else the cache may bounce the write + // because it thinks the document is too big. INT64_MAX + // is a special case for the max document size code + // in the cache + if (c_write != INT64_MAX) { + c_write -= c->skip_bytes; + } + // Fix for problems with not chunked content being chunked and + // not sending the entire data. The content length grows when + // it is being chunked. + if (p->do_chunking == true) { + c_write = INT64_MAX; + } + } + if (c_write == 0) { + // Nothing to do, call back the cleanup handlers + c->write_vio = nullptr; + consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + } else { + // In the client half close case, all the data that will be sent + // from the client is already in the buffer. Go ahead and set + // the amount to read since we know it. We will forward the FIN + // to the server on VC_EVENT_WRITE_COMPLETE. + if (p->vc_type == HT_HTTP_CLIENT) { + ProxyClientTransaction *ua_vc = static_cast<ProxyClientTransaction *>(p->vc); + if (ua_vc->get_half_close_flag()) { + int tmp = c->buffer_reader->read_avail(); + if (tmp < c_write) { + c_write = tmp; + } + p->alive = false; + p->handler_state = HTTP_SM_POST_SUCCESS; + } + } + // Start the writes now that we know we will consume all the initial data + c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader); + ink_assert(c_write > 0); + } + } if (p->alive) { ink_assert(producer_n >= 0); @@ -1184,8 +1193,8 @@ HttpTunnel::producer_handler(int event, HttpTunnelProducer *p) } } // end of added logic for partial copy of POST - Debug("http_redirect", "[HttpTunnel::producer_handler] enable_redirection: [%d %d %d] event: %d", p->alive == true, - sm->enable_redirection, (p->self_consumer && p->self_consumer->alive == true), event); + Debug("http_redirect", "[HttpTunnel::producer_handler] enable_redirection: [%d %d %d] event: %d, state: %d", p->alive == true, + sm->enable_redirection, (p->self_consumer && p->self_consumer->alive == true), event, p->chunked_handler.state); switch (event) { case VC_EVENT_READ_READY: @@ -1470,21 +1479,18 @@ HttpTunnel::chain_abort_all(HttpTunnelProducer *p) } } -// void HttpTunnel::chain_finish_internal(HttpTunnelProducer* p) // -// Internal function for finishing all consumers. Takes -// chain argument about where to finish just immediate -// consumer or all those downstream +// Determine the number of bytes a consumer should read from a producer // -void -HttpTunnel::finish_all_internal(HttpTunnelProducer *p, bool chain) +int64_t +HttpTunnel::final_consumer_bytes_to_write(HttpTunnelProducer *p, HttpTunnelConsumer *c) { - ink_assert(p->alive == false); - HttpTunnelConsumer *c = p->consumer_list.head; - int64_t total_bytes = 0; - TunnelChunkingAction_t action = p->chunking_action; - - while (c) { + int64_t total_bytes = 0; + int64_t consumer_n = 0; + if (p->alive) { + consumer_n = INT64_MAX; + } else { + TunnelChunkingAction_t action = p->chunking_action; if (c->alive) { if (c->vc_type == HT_CACHE_WRITE) { switch (action) { @@ -1503,12 +1509,48 @@ HttpTunnel::finish_all_internal(HttpTunnelProducer *p, bool chain) total_bytes = p->chunked_handler.skip_bytes + p->chunked_handler.chunked_size; } else if (action == TCA_DECHUNK_CONTENT) { total_bytes = p->chunked_handler.skip_bytes + p->chunked_handler.dechunked_size; + } else if (action == TCA_PASSTHRU_CHUNKED_CONTENT) { + total_bytes = p->bytes_read + p->init_bytes_done; } else { total_bytes = p->bytes_read + p->init_bytes_done; } + consumer_n = total_bytes - c->skip_bytes; + } + } + return consumer_n; +} +// +// void HttpTunnel::finish_all_internal(HttpTunnelProducer* p) +// +// Internal function for finishing all consumers. Takes +// chain argument about where to finish just immediate +// consumer or all those downstream +// +void +HttpTunnel::finish_all_internal(HttpTunnelProducer *p, bool chain) +{ + ink_assert(p->alive == false); + HttpTunnelConsumer *c = p->consumer_list.head; + int64_t total_bytes = 0; + TunnelChunkingAction_t action = p->chunking_action; + + if (action == TCA_PASSTHRU_CHUNKED_CONTENT) { + // if the only chunked data was in the initial read, make sure we don't consume too much + if (p->bytes_read == 0) { + int num_read = p->buffer_start->read_avail() - p->chunked_handler.chunked_reader->read_avail(); + if (num_read < p->init_bytes_done) { + p->init_bytes_done = num_read; + } + } + } + + while (c) { + if (c->alive) { if (c->write_vio) { - c->write_vio->nbytes = total_bytes - c->skip_bytes; + // Adjust the number of bytes to write in the case of + // a completed unlimited producer + c->write_vio->nbytes = final_consumer_bytes_to_write(p, c); ink_assert(c->write_vio->nbytes >= 0); if (c->write_vio->nbytes < 0) { @@ -1525,7 +1567,7 @@ HttpTunnel::finish_all_internal(HttpTunnelProducer *p, bool chain) // is nothing to do. Check to see if there is // nothing to do and take the appripriate // action - if (c->write_vio && c->write_vio->nbytes == c->write_vio->ndone) { + if (c->write_vio && c->alive && c->write_vio->nbytes == c->write_vio->ndone) { consumer_handler(VC_EVENT_WRITE_COMPLETE, c); } } diff --git a/proxy/http/HttpTunnel.h b/proxy/http/HttpTunnel.h index e47160d..a432f3f 100644 --- a/proxy/http/HttpTunnel.h +++ b/proxy/http/HttpTunnel.h @@ -312,6 +312,7 @@ public: void chain_abort_all(HttpTunnelProducer *p); void abort_cache_write_finish_others(HttpTunnelProducer *p); void append_message_to_producer_buffer(HttpTunnelProducer *p, const char *msg, int64_t msg_len); + int64_t final_consumer_bytes_to_write(HttpTunnelProducer *p, HttpTunnelConsumer *c); /** Mark a producer and consumer as the same underlying object.