You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2014/09/22 03:19:33 UTC

git commit: TS-3073: Forward FIN from client to server in tr-pass handling.

Repository: trafficserver
Updated Branches:
  refs/heads/master 9e46e4e19 -> 514c9e1a6


TS-3073: Forward FIN from client to server in tr-pass handling.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/514c9e1a
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/514c9e1a
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/514c9e1a

Branch: refs/heads/master
Commit: 514c9e1a6a77bf0b7b956be9f9cc79306e98dedc
Parents: 9e46e4e
Author: Alan M. Carroll <am...@apache.org>
Authored: Sun Sep 21 17:25:25 2014 -0500
Committer: Alan M. Carroll <am...@apache.org>
Committed: Sun Sep 21 20:18:49 2014 -0500

----------------------------------------------------------------------
 CHANGES                        |  2 ++
 proxy/http/HttpClientSession.h |  1 +
 proxy/http/HttpSM.cc           | 22 ++++++++++++++++------
 proxy/http/HttpTunnel.cc       | 11 +++++++++++
 4 files changed, 30 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index bccfff6..b401177 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.2.0
 
+  *) [TS-3073] Fix FIN forwarding issue with transparent pass-through.
+
   *) [TS-3059] Add the TSTextLogObjectRollingSizeMbSet API function.
    Author: Brian Rectanus <br...@qualys.com>
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpClientSession.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpClientSession.h b/proxy/http/HttpClientSession.h
index ef0768d..7da29d9 100644
--- a/proxy/http/HttpClientSession.h
+++ b/proxy/http/HttpClientSession.h
@@ -72,6 +72,7 @@ public:
   void new_transaction();
 
   void set_half_close_flag() { half_close = true; };
+  bool get_half_close_flag() { return half_close; };
   virtual void release(IOBufferReader * r);
   NetVConnection *get_netvc() const { return client_vc;  };
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index eadfe1d..3dde3c3 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -670,7 +670,10 @@ HttpSM::state_read_client_request_header(int event, void *data)
     state = PARSE_ERROR;
   }
 
-  if (event == VC_EVENT_READ_READY &&
+  // We need to handle EOS as well as READ_READY because the client
+  // may have sent all of the data already followed by a fIN and that
+  // should be OK.
+  if ((event == VC_EVENT_READ_READY || event == VC_EVENT_EOS) &&
       state == PARSE_ERROR &&
       is_transparent_passthrough_allowed() &&
       ua_raw_buffer_reader != NULL) {
@@ -686,6 +689,12 @@ HttpSM::state_read_client_request_header(int event, void *data)
 
       /* establish blind tunnel */
       setup_blind_tunnel_port();
+
+      // Setting half close means we will send the FIN when we've written all of the data.
+      if (event == VC_EVENT_EOS) {
+        this->set_ua_half_close_flag();      
+        t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
+      }
       return 0;
   }
 
@@ -3590,11 +3599,6 @@ HttpSM::tunnel_handler_ssl_consumer(int event, HttpTunnelConsumer * c)
     c->write_success = true;
     if (c->self_producer->alive == true) {
       c->vc->do_io_shutdown(IO_SHUTDOWN_WRITE);
-      if (!c->producer->alive) {
-        tunnel.close_vc(c);
-        tunnel.local_finish_all(c->self_producer);
-        break;
-      }
     } else {
       c->vc->do_io_close();
     }
@@ -6393,6 +6397,12 @@ HttpSM::setup_blind_tunnel(bool send_response_hdr)
   server_entry->in_tunnel = true;
 
   tunnel.tunnel_run();
+
+  // If we're half closed, we got a FIN from the client. Forward it on to the origin server
+  // now that we have the tunnel operational.
+  if (ua_session->get_half_close_flag()) {
+    p_ua->vc->do_io_shutdown(IO_SHUTDOWN_READ);
+  }
 }
 
 void

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpTunnel.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index d71f6c8..d7f362c 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -891,6 +891,17 @@ HttpTunnel::producer_run(HttpTunnelProducer * p)
       c->write_vio = NULL;
       consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
     } else {
+      // In the client half close case, all the buffer 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 send the FIN
+      // to the server on VC_EVENT_WRITE_COMPLETE.
+      if (p->vc_type == HT_HTTP_CLIENT) {
+        if (static_cast<HttpClientSession *>(p->vc)->get_half_close_flag()) {
+          c_write = c->buffer_reader->read_avail();
+          p->alive = false;
+        }
+      }
+
       c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader);
       ink_assert(c_write > 0);
     }


Re: git commit: TS-3073: Forward FIN from client to server in tr-pass handling.

Posted by James Peach <jp...@apache.org>.
On Sep 21, 2014, at 6:19 PM, amc@apache.org wrote:

> Repository: trafficserver
> Updated Branches:
>  refs/heads/master 9e46e4e19 -> 514c9e1a6
> 
> 
> TS-3073: Forward FIN from client to server in tr-pass handling.
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/514c9e1a
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/514c9e1a
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/514c9e1a
> 
> Branch: refs/heads/master
> Commit: 514c9e1a6a77bf0b7b956be9f9cc79306e98dedc
> Parents: 9e46e4e
> Author: Alan M. Carroll <am...@apache.org>
> Authored: Sun Sep 21 17:25:25 2014 -0500
> Committer: Alan M. Carroll <am...@apache.org>
> Committed: Sun Sep 21 20:18:49 2014 -0500
> 
> ----------------------------------------------------------------------
> CHANGES                        |  2 ++
> proxy/http/HttpClientSession.h |  1 +
> proxy/http/HttpSM.cc           | 22 ++++++++++++++++------
> proxy/http/HttpTunnel.cc       | 11 +++++++++++
> 4 files changed, 30 insertions(+), 6 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index bccfff6..b401177 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,8 @@
>                                                          -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 5.2.0
> 
> +  *) [TS-3073] Fix FIN forwarding issue with transparent pass-through.
> +
>   *) [TS-3059] Add the TSTextLogObjectRollingSizeMbSet API function.
>    Author: Brian Rectanus <br...@qualys.com>
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpClientSession.h
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpClientSession.h b/proxy/http/HttpClientSession.h
> index ef0768d..7da29d9 100644
> --- a/proxy/http/HttpClientSession.h
> +++ b/proxy/http/HttpClientSession.h
> @@ -72,6 +72,7 @@ public:
>   void new_transaction();
> 
>   void set_half_close_flag() { half_close = true; };
> +  bool get_half_close_flag() { return half_close; };


bool get_half_close_flag() const { ... };

>   virtual void release(IOBufferReader * r);
>   NetVConnection *get_netvc() const { return client_vc;  };
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpSM.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
> index eadfe1d..3dde3c3 100644
> --- a/proxy/http/HttpSM.cc
> +++ b/proxy/http/HttpSM.cc
> @@ -670,7 +670,10 @@ HttpSM::state_read_client_request_header(int event, void *data)
>     state = PARSE_ERROR;
>   }
> 
> -  if (event == VC_EVENT_READ_READY &&
> +  // We need to handle EOS as well as READ_READY because the client
> +  // may have sent all of the data already followed by a fIN and that
> +  // should be OK.
> +  if ((event == VC_EVENT_READ_READY || event == VC_EVENT_EOS) &&
>       state == PARSE_ERROR &&
>       is_transparent_passthrough_allowed() &&
>       ua_raw_buffer_reader != NULL) {
> @@ -686,6 +689,12 @@ HttpSM::state_read_client_request_header(int event, void *data)
> 
>       /* establish blind tunnel */
>       setup_blind_tunnel_port();
> +
> +      // Setting half close means we will send the FIN when we've written all of the data.
> +      if (event == VC_EVENT_EOS) {
> +        this->set_ua_half_close_flag();      
> +        t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
> +      }
>       return 0;
>   }
> 
> @@ -3590,11 +3599,6 @@ HttpSM::tunnel_handler_ssl_consumer(int event, HttpTunnelConsumer * c)
>     c->write_success = true;
>     if (c->self_producer->alive == true) {
>       c->vc->do_io_shutdown(IO_SHUTDOWN_WRITE);
> -      if (!c->producer->alive) {
> -        tunnel.close_vc(c);
> -        tunnel.local_finish_all(c->self_producer);
> -        break;
> -      }
>     } else {
>       c->vc->do_io_close();
>     }
> @@ -6393,6 +6397,12 @@ HttpSM::setup_blind_tunnel(bool send_response_hdr)
>   server_entry->in_tunnel = true;
> 
>   tunnel.tunnel_run();
> +
> +  // If we're half closed, we got a FIN from the client. Forward it on to the origin server
> +  // now that we have the tunnel operational.
> +  if (ua_session->get_half_close_flag()) {
> +    p_ua->vc->do_io_shutdown(IO_SHUTDOWN_READ);
> +  }
> }
> 
> void
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpTunnel.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
> index d71f6c8..d7f362c 100644
> --- a/proxy/http/HttpTunnel.cc
> +++ b/proxy/http/HttpTunnel.cc
> @@ -891,6 +891,17 @@ HttpTunnel::producer_run(HttpTunnelProducer * p)
>       c->write_vio = NULL;
>       consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
>     } else {
> +      // In the client half close case, all the buffer 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 send the FIN
> +      // to the server on VC_EVENT_WRITE_COMPLETE.
> +      if (p->vc_type == HT_HTTP_CLIENT) {
> +        if (static_cast<HttpClientSession *>(p->vc)->get_half_close_flag()) {
> +          c_write = c->buffer_reader->read_avail();
> +          p->alive = false;
> +        }
> +      }
> +
>       c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader);
>       ink_assert(c_write > 0);
>     }
> 


Re: git commit: TS-3073: Forward FIN from client to server in tr-pass handling.

Posted by James Peach <jp...@apache.org>.
On Sep 21, 2014, at 6:19 PM, amc@apache.org wrote:

> Repository: trafficserver
> Updated Branches:
>  refs/heads/master 9e46e4e19 -> 514c9e1a6
> 
> 
> TS-3073: Forward FIN from client to server in tr-pass handling.
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/514c9e1a
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/514c9e1a
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/514c9e1a
> 
> Branch: refs/heads/master
> Commit: 514c9e1a6a77bf0b7b956be9f9cc79306e98dedc
> Parents: 9e46e4e
> Author: Alan M. Carroll <am...@apache.org>
> Authored: Sun Sep 21 17:25:25 2014 -0500
> Committer: Alan M. Carroll <am...@apache.org>
> Committed: Sun Sep 21 20:18:49 2014 -0500
> 
> ----------------------------------------------------------------------
> CHANGES                        |  2 ++
> proxy/http/HttpClientSession.h |  1 +
> proxy/http/HttpSM.cc           | 22 ++++++++++++++++------
> proxy/http/HttpTunnel.cc       | 11 +++++++++++
> 4 files changed, 30 insertions(+), 6 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index bccfff6..b401177 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,8 @@
>                                                          -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 5.2.0
> 
> +  *) [TS-3073] Fix FIN forwarding issue with transparent pass-through.
> +
>   *) [TS-3059] Add the TSTextLogObjectRollingSizeMbSet API function.
>    Author: Brian Rectanus <br...@qualys.com>
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpClientSession.h
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpClientSession.h b/proxy/http/HttpClientSession.h
> index ef0768d..7da29d9 100644
> --- a/proxy/http/HttpClientSession.h
> +++ b/proxy/http/HttpClientSession.h
> @@ -72,6 +72,7 @@ public:
>   void new_transaction();
> 
>   void set_half_close_flag() { half_close = true; };
> +  bool get_half_close_flag() { return half_close; };


bool get_half_close_flag() const { ... };

>   virtual void release(IOBufferReader * r);
>   NetVConnection *get_netvc() const { return client_vc;  };
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpSM.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
> index eadfe1d..3dde3c3 100644
> --- a/proxy/http/HttpSM.cc
> +++ b/proxy/http/HttpSM.cc
> @@ -670,7 +670,10 @@ HttpSM::state_read_client_request_header(int event, void *data)
>     state = PARSE_ERROR;
>   }
> 
> -  if (event == VC_EVENT_READ_READY &&
> +  // We need to handle EOS as well as READ_READY because the client
> +  // may have sent all of the data already followed by a fIN and that
> +  // should be OK.
> +  if ((event == VC_EVENT_READ_READY || event == VC_EVENT_EOS) &&
>       state == PARSE_ERROR &&
>       is_transparent_passthrough_allowed() &&
>       ua_raw_buffer_reader != NULL) {
> @@ -686,6 +689,12 @@ HttpSM::state_read_client_request_header(int event, void *data)
> 
>       /* establish blind tunnel */
>       setup_blind_tunnel_port();
> +
> +      // Setting half close means we will send the FIN when we've written all of the data.
> +      if (event == VC_EVENT_EOS) {
> +        this->set_ua_half_close_flag();      
> +        t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
> +      }
>       return 0;
>   }
> 
> @@ -3590,11 +3599,6 @@ HttpSM::tunnel_handler_ssl_consumer(int event, HttpTunnelConsumer * c)
>     c->write_success = true;
>     if (c->self_producer->alive == true) {
>       c->vc->do_io_shutdown(IO_SHUTDOWN_WRITE);
> -      if (!c->producer->alive) {
> -        tunnel.close_vc(c);
> -        tunnel.local_finish_all(c->self_producer);
> -        break;
> -      }
>     } else {
>       c->vc->do_io_close();
>     }
> @@ -6393,6 +6397,12 @@ HttpSM::setup_blind_tunnel(bool send_response_hdr)
>   server_entry->in_tunnel = true;
> 
>   tunnel.tunnel_run();
> +
> +  // If we're half closed, we got a FIN from the client. Forward it on to the origin server
> +  // now that we have the tunnel operational.
> +  if (ua_session->get_half_close_flag()) {
> +    p_ua->vc->do_io_shutdown(IO_SHUTDOWN_READ);
> +  }
> }
> 
> void
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/514c9e1a/proxy/http/HttpTunnel.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
> index d71f6c8..d7f362c 100644
> --- a/proxy/http/HttpTunnel.cc
> +++ b/proxy/http/HttpTunnel.cc
> @@ -891,6 +891,17 @@ HttpTunnel::producer_run(HttpTunnelProducer * p)
>       c->write_vio = NULL;
>       consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
>     } else {
> +      // In the client half close case, all the buffer 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 send the FIN
> +      // to the server on VC_EVENT_WRITE_COMPLETE.
> +      if (p->vc_type == HT_HTTP_CLIENT) {
> +        if (static_cast<HttpClientSession *>(p->vc)->get_half_close_flag()) {
> +          c_write = c->buffer_reader->read_avail();
> +          p->alive = false;
> +        }
> +      }
> +
>       c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader);
>       ink_assert(c_write > 0);
>     }
>