You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2018/01/30 16:00:13 UTC

[trafficserver] branch master updated: Add support to chunk post body's that come from H2 client without content length.

This is an automated email from the ASF dual-hosted git repository.

shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 429ae17  Add support to chunk post body's that come from H2 client without content length.
429ae17 is described below

commit 429ae17f7a6c345cc731eb3b96270e02aca54e35
Author: Susan Hinrichs <sh...@apache.org>
AuthorDate: Fri Jan 26 14:39:24 2018 -0600

    Add support to chunk post body's that come from H2 client without content length.
---
 proxy/ProxyClientSession.h                 |  6 +++
 proxy/ProxyClientTransaction.h             |  6 +++
 proxy/http/Http1ClientSession.h            | 62 ++++++++++++++++--------------
 proxy/http/HttpSM.cc                       |  9 ++++-
 proxy/http/HttpTransact.cc                 | 15 +++++++-
 proxy/http/HttpTunnel.cc                   | 11 +++++-
 proxy/http2/Http2Stream.cc                 |  8 ++--
 proxy/http2/Http2Stream.h                  |  2 +-
 tests/gold_tests/h2/gold/post_chunked.gold |  1 +
 tests/gold_tests/h2/http2.test.py          | 14 +++++++
 10 files changed, 96 insertions(+), 38 deletions(-)

diff --git a/proxy/ProxyClientSession.h b/proxy/ProxyClientSession.h
index 89684de..9eaaf63 100644
--- a/proxy/ProxyClientSession.h
+++ b/proxy/ProxyClientSession.h
@@ -138,6 +138,12 @@ public:
     return false;
   }
 
+  virtual bool
+  is_chunked_encoding_supported() const
+  {
+    return false;
+  }
+
   // Override if your session protocol cares.
   virtual void
   set_half_close_flag(bool flag)
diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h
index 458e790..224df05 100644
--- a/proxy/ProxyClientTransaction.h
+++ b/proxy/ProxyClientTransaction.h
@@ -73,6 +73,12 @@ public:
     return parent ? parent->is_transparent_passthrough_allowed() : false;
   }
 
+  virtual bool
+  is_chunked_encoding_supported() const
+  {
+    return parent ? parent->is_chunked_encoding_supported() : false;
+  }
+
   void
   set_half_close_flag(bool flag)
   {
diff --git a/proxy/http/Http1ClientSession.h b/proxy/http/Http1ClientSession.h
index de56f55..779c3ec 100644
--- a/proxy/http/Http1ClientSession.h
+++ b/proxy/http/Http1ClientSession.h
@@ -55,47 +55,53 @@ public:
   Http1ClientSession();
 
   // Implement ProxyClientSession interface.
-  virtual void destroy();
-  virtual void free();
+  void destroy() override;
+  void free() override;
   void release_transaction();
 
-  virtual void
-  start()
+  void
+  start() override
   {
     // Troll for data to get a new transaction
     this->release(&trans);
   }
 
-  void new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOBufferReader *reader, bool backdoor);
+  void new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOBufferReader *reader, bool backdoor) override;
 
   // Implement VConnection interface.
-  virtual VIO *do_io_read(Continuation *c, int64_t nbytes = INT64_MAX, MIOBuffer *buf = 0);
-  virtual VIO *do_io_write(Continuation *c = NULL, int64_t nbytes = INT64_MAX, IOBufferReader *buf = 0, bool owner = false);
+  VIO *do_io_read(Continuation *c, int64_t nbytes = INT64_MAX, MIOBuffer *buf = 0) override;
+  VIO *do_io_write(Continuation *c = NULL, int64_t nbytes = INT64_MAX, IOBufferReader *buf = 0, bool owner = false) override;
 
-  virtual void do_io_close(int lerrno = -1);
-  virtual void do_io_shutdown(ShutdownHowTo_t howto);
-  virtual void reenable(VIO *vio);
+  void do_io_close(int lerrno = -1) override;
+  void do_io_shutdown(ShutdownHowTo_t howto) override;
+  void reenable(VIO *vio) override;
 
   void
-  set_half_close_flag(bool flag)
+  set_half_close_flag(bool flag) override
   {
     half_close = flag;
   }
 
   bool
-  get_half_close_flag() const
+  get_half_close_flag() const override
   {
     return half_close;
   }
 
-  virtual NetVConnection *
-  get_netvc() const
+  bool
+  is_chunked_encoding_supported() const override
+  {
+    return true;
+  }
+
+  NetVConnection *
+  get_netvc() const override
   {
     return client_vc;
   }
 
-  virtual void
-  release_netvc()
+  void
+  release_netvc() override
   {
     // Make sure the vio's are also released to avoid
     // later surprises in inactivity timeout
@@ -108,7 +114,7 @@ public:
   }
 
   int
-  get_transact_count() const
+  get_transact_count() const override
   {
     return transact_count;
   }
@@ -120,7 +126,7 @@ public:
   }
 
   // Indicate we are done with a transaction
-  virtual void release(ProxyClientTransaction *trans);
+  void release(ProxyClientTransaction *trans) override;
 
   virtual uint16_t
   get_outbound_port() const
@@ -140,43 +146,43 @@ public:
     return outbound_ip6;
   }
 
-  virtual void attach_server_session(HttpServerSession *ssession, bool transaction_done = true);
+  void attach_server_session(HttpServerSession *ssession, bool transaction_done = true) override;
 
-  virtual HttpServerSession *
-  get_server_session() const
+  HttpServerSession *
+  get_server_session() const override
   {
     return bound_ss;
   }
 
   void
-  set_active_timeout(ink_hrtime timeout_in)
+  set_active_timeout(ink_hrtime timeout_in) override
   {
     if (client_vc)
       client_vc->set_active_timeout(timeout_in);
   }
 
   void
-  set_inactivity_timeout(ink_hrtime timeout_in)
+  set_inactivity_timeout(ink_hrtime timeout_in) override
   {
     if (client_vc)
       client_vc->set_inactivity_timeout(timeout_in);
   }
 
   void
-  cancel_inactivity_timeout()
+  cancel_inactivity_timeout() override
   {
     if (client_vc)
       client_vc->cancel_inactivity_timeout();
   }
 
-  virtual const char *
-  get_protocol_string() const
+  const char *
+  get_protocol_string() const override
   {
     return "http";
   }
 
-  virtual bool
-  is_transparent_passthrough_allowed() const
+  bool
+  is_transparent_passthrough_allowed() const override
   {
     return f_transparent_passthrough;
   }
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 95f9d07..d4467d3 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5615,8 +5615,15 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type)
     break;
   }
 
+  // The user agent may support chunked (HTTP/1.1) or not (HTTP/2)
+  // In either case, the server will support chunked (HTTP/1.1)
   if (chunked) {
-    tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT);
+    if (ua_txn->is_chunked_encoding_supported()) {
+      tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT);
+    } else {
+      tunnel.set_producer_chunking_action(p, 0, TCA_CHUNK_CONTENT);
+      tunnel.set_producer_chunking_size(p, 0);
+    }
   }
 
   ua_txn->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in));
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index bb9167b..6ad5767 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -5128,8 +5128,19 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
     if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
         (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
         s->client_info.transfer_encoding != CHUNKED_ENCODING) {
-      if ((s->txn_conf->post_check_content_length_enabled) && !incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) {
-        return NO_POST_CONTENT_LENGTH;
+      if (!incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) {
+        // In normal operation there will always be a ua_txn at this point, but in one of the -R1  regression tests a request is
+        // created
+        // independent of a transaction and this method is called, so we must null check
+        if (s->txn_conf->post_check_content_length_enabled &&
+            (!s->state_machine->ua_txn || s->state_machine->ua_txn->is_chunked_encoding_supported())) {
+          return NO_POST_CONTENT_LENGTH;
+        } else {
+          // Stuff in a TE setting so we treat this as chunked, sort of.
+          s->client_info.transfer_encoding = HttpTransact::CHUNKED_ENCODING;
+          incoming_hdr->value_append(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING, HTTP_VALUE_CHUNKED, HTTP_LEN_CHUNKED,
+                                     true);
+        }
       }
       if (HTTP_UNDEFINED_CL == s->hdr_info.request_content_length) {
         return INVALID_POST_CONTENT_LENGTH;
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index aecf2f5..c5f1fa5 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -1047,8 +1047,15 @@ HttpTunnel::producer_handler_dechunked(int event, HttpTunnelProducer *p)
   case HTTP_TUNNEL_EVENT_PRECOMPLETE:
   case VC_EVENT_EOS:
     p->last_event = p->chunked_handler.last_server_event = event;
-    // TODO: Should we check the return code?
-    p->chunked_handler.generate_chunked_content();
+    if (p->chunked_handler.generate_chunked_content()) { // We are done, make sure the consumer is activated
+      HttpTunnelConsumer *c;
+      for (c = p->consumer_list.head; c; c = c->link.next) {
+        if (c->alive) {
+          c->write_vio->nbytes = p->chunked_handler.chunked_size;
+          // consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
+        }
+      }
+    }
     break;
   };
   // Since we will consume all the data if the server is actually finished
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 81cf6b6..26d468e 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -289,7 +289,7 @@ Http2Stream::do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf)
 
   // Is there already data in the request_buffer?  If so, copy it over and then
   // schedule a READ_READY or READ_COMPLETE event after we return.
-  update_read_request(nbytes, false);
+  update_read_request(nbytes, false, true);
 
   return &read_vio;
 }
@@ -458,7 +458,7 @@ Http2Stream::send_tracked_event(Event *event, int send_event, VIO *vio)
 }
 
 void
-Http2Stream::update_read_request(int64_t read_len, bool call_update)
+Http2Stream::update_read_request(int64_t read_len, bool call_update, bool check_eos)
 {
   if (closed || parent == nullptr || current_reader == nullptr || read_vio.mutex == nullptr) {
     return;
@@ -483,10 +483,10 @@ Http2Stream::update_read_request(int64_t read_len, bool call_update)
       }
       if (num_to_read > 0) {
         int bytes_added = read_vio.buffer.writer()->write(request_reader, num_to_read);
-        if (bytes_added > 0) {
+        if (bytes_added > 0 || (check_eos && recv_end_stream)) {
           request_reader->consume(bytes_added);
           read_vio.ndone += bytes_added;
-          int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
+          int send_event = (read_vio.nbytes == read_vio.ndone || recv_end_stream) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
           if (call_update) { // Safe to call vio handler directly
             inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
             if (read_vio._cont && this->current_reader) {
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index b6f9cf9..884a0c0 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -150,7 +150,7 @@ public:
   void initiating_close();
   void terminate_if_possible();
   void do_io_shutdown(ShutdownHowTo_t) override {}
-  void update_read_request(int64_t read_len, bool send_update);
+  void update_read_request(int64_t read_len, bool send_update, bool check_eos = false);
   bool update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
   void reenable(VIO *vio) override;
   virtual void transaction_done() override;
diff --git a/tests/gold_tests/h2/gold/post_chunked.gold b/tests/gold_tests/h2/gold/post_chunked.gold
new file mode 100644
index 0000000..11f11f9
--- /dev/null
+++ b/tests/gold_tests/h2/gold/post_chunked.gold
@@ -0,0 +1 @@
+0123456789
diff --git a/tests/gold_tests/h2/http2.test.py b/tests/gold_tests/h2/http2.test.py
index 5fffac2..f98c6e2 100644
--- a/tests/gold_tests/h2/http2.test.py
+++ b/tests/gold_tests/h2/http2.test.py
@@ -50,6 +50,13 @@ server.addResponse("sessionlog.json",
                    {"headers": "GET /bigfile HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""},
                    {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nCache-Control: max-age=3600\r\nContent-Length: 191414\r\n\r\n", "timestamp": "1469733493.993", "body": ""})
 
+post_body = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
+server.addResponse("sessionlog.jason",
+                   {"headers": "POST /postchunked HTTP/1.1\r\nHost: www.example.com\r\n\r\n", 
+                    "timestamp": "1469733493.993",
+                     "body": post_body},
+                   {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nContent-Length: 10\r\n\r\n", "timestamp": "1469733493.993", "body": "0123456789"})
+
 server.addResponse("sessionlog.json", request_header2, response_header2)
 # add ssl materials like key, certificates for the server
 ts.addSSLfile("ssl/server.pem")
@@ -120,3 +127,10 @@ tr.Processes.Default.Command = 'python3 h2active_timeout.py -p {0}'.format(ts.Va
 tr.Processes.Default.ReturnCode = 0
 tr.Processes.Default.Streams.All = "gold/active_timeout.gold"
 tr.StillRunningAfter = server
+
+# Test Case 6: Post with chunked body
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -k -H "Transfer-Encoding: chunked" -d "{0}" https://127.0.0.1:{1}/postchunked'.format( post_body, ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.All = "gold/post_chunked.gold"
+tr.StillRunningAfter = server

-- 
To stop receiving notification emails like this one, please contact
shinrich@apache.org.