You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2020/02/05 23:19:41 UTC
[trafficserver] branch 7.1.x updated: Fix chunked processing
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/7.1.x by this push:
new 3b6dc41 Fix chunked processing
3b6dc41 is described below
commit 3b6dc411c2321d55003c6b7151211e3cd51ceb49
Author: Susan Hinrichs <sh...@verizonmedia.com>
AuthorDate: Wed Feb 5 19:17:50 2020 +0000
Fix chunked processing
---
iocore/net/UnixNetVConnection.cc | 2 +-
proxy/ProxyClientSession.h | 6 ++
proxy/ProxyClientTransaction.h | 6 ++
proxy/http/Http1ClientSession.h | 6 ++
proxy/http/HttpTransact.cc | 3 +-
proxy/http/HttpTunnel.cc | 160 ++++++++++++++++++++++++---------------
proxy/http/HttpTunnel.h | 1 +
7 files changed, 122 insertions(+), 62 deletions(-)
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index a30200b..7cc2363 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -985,7 +985,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/ProxyClientSession.h b/proxy/ProxyClientSession.h
index 22190e6..32100e3 100644
--- a/proxy/ProxyClientSession.h
+++ b/proxy/ProxyClientSession.h
@@ -65,6 +65,12 @@ public:
virtual NetVConnection *get_netvc() const = 0;
+ virtual bool
+ is_chunked_encoding_supported() const
+ {
+ return false;
+ }
+
virtual int get_transact_count() const = 0;
virtual const char *get_protocol_string() const = 0;
diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h
index 5398c9e..1656700 100644
--- a/proxy/ProxyClientTransaction.h
+++ b/proxy/ProxyClientTransaction.h
@@ -244,6 +244,12 @@ public:
return parent ? parent->populate_protocol(result, size) : 0;
}
+ virtual bool
+ is_chunked_encoding_supported() const
+ {
+ return parent ? parent->is_chunked_encoding_supported() : false;
+ }
+
virtual const char *
protocol_contains(ts::StringView tag_prefix) const
{
diff --git a/proxy/http/Http1ClientSession.h b/proxy/http/Http1ClientSession.h
index 37a626e..8f9e20b 100644
--- a/proxy/http/Http1ClientSession.h
+++ b/proxy/http/Http1ClientSession.h
@@ -168,6 +168,12 @@ public:
return "http";
}
+ bool
+ is_chunked_encoding_supported() const override
+ {
+ return true;
+ }
+
private:
Http1ClientSession(Http1ClientSession &);
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 073587d..156441d 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -7103,8 +7103,7 @@ 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)))) &&
+ (s->txn_conf->chunking_enabled == 1 && s->state_machine->ua_session->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 aa5d1a0..561c55e 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -899,11 +899,10 @@ 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
ink_release_assert(p->num_consumers > 0);
- 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) {
@@ -934,45 +933,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
@@ -1028,7 +988,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);
@@ -1195,8 +1204,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:
@@ -1478,21 +1487,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) {
@@ -1511,12 +1517,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) {
@@ -1533,7 +1575,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 17df32a..76a6755 100644
--- a/proxy/http/HttpTunnel.h
+++ b/proxy/http/HttpTunnel.h
@@ -319,6 +319,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.