From 4a9c564ff7727cab7929a29e38f5ff5ae0c62c62 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Fri, 22 Jul 2016 15:17:50 +0900 Subject: [PATCH] TS-4694: Some refactoring after SPDY is removed - Remove PluginIdentity class from base classes of Http2ClientSession - Add get_protocol_string() to ProxyClientSession to identify if the session is HTTP/2 or HTTP/1.x - Add "client_protocol" to HttpSM to track client protocol versions - Drop HTTP/0.9 support from cqpv (HTTP/0.9 is already dropped by TS-3327) --- proxy/ProxyClientSession.h | 1 + proxy/ProxyClientTransaction.cc | 16 ---------------- proxy/ProxyClientTransaction.h | 6 +++++- proxy/http/Http1ClientSession.h | 6 ++++++ proxy/http/Http1ClientTransaction.h | 5 ----- proxy/http/HttpSM.cc | 3 +++ proxy/http/HttpSM.h | 1 + proxy/http2/Http2ClientSession.cc | 12 ------------ proxy/http2/Http2ClientSession.h | 11 +++++++---- proxy/http2/Http2Stream.h | 6 ------ proxy/logging/LogAccessHttp.cc | 22 ++++++++++------------ 11 files changed, 33 insertions(+), 56 deletions(-) diff --git a/proxy/ProxyClientSession.h b/proxy/ProxyClientSession.h index efa2da13a15..b92e0609613 100644 --- a/proxy/ProxyClientSession.h +++ b/proxy/ProxyClientSession.h @@ -175,6 +175,7 @@ class ProxyClientSession : public VConnection cancel_inactivity_timeout() { } + virtual const char *get_protocol_string() const = 0; protected: // XXX Consider using a bitwise flags variable for the following flags, so that we can make the best diff --git a/proxy/ProxyClientTransaction.cc b/proxy/ProxyClientTransaction.cc index a7a83674352..e1bad21f43b 100644 --- a/proxy/ProxyClientTransaction.cc +++ b/proxy/ProxyClientTransaction.cc @@ -52,22 +52,6 @@ ProxyClientTransaction::new_transaction() DebugHttpTxn("[%" PRId64 "] Starting transaction %d using sm [%" PRId64 "]", parent->connection_id(), parent->get_transact_count(), current_reader->sm_id); - // This is a temporary hack until we get rid of SPDY and can use virutal methods entirely - // to track protocol. - PluginIdentity *pi = dynamic_cast(this->get_netvc()); - if (pi) { - current_reader->plugin_tag = pi->getPluginTag(); - current_reader->plugin_id = pi->getPluginId(); - } else { - const char *protocol_str = this->get_protocol_string(); - // We don't set the plugin_tag for http, though in future we should probably log http as protocol - if (strlen(protocol_str) != 4 || strncmp("http", protocol_str, 4)) { - current_reader->plugin_tag = protocol_str; - // Since there is no more plugin, there is no plugin id for http/2 - // We are copying along the plugin_tag as a standin for protocol name for logging - // and to detect a case in HttpTransaction (TS-3954) - } - } current_reader->attach_client_session(this, sm_reader); } diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h index f692f5977ec..9dac03d3775 100644 --- a/proxy/ProxyClientTransaction.h +++ b/proxy/ProxyClientTransaction.h @@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection virtual bool allow_half_open() const = 0; - virtual const char *get_protocol_string() const = 0; + virtual const char * + get_protocol_string() + { + return parent ? parent->get_protocol_string() : NULL; + } void set_restart_immediate(bool val) diff --git a/proxy/http/Http1ClientSession.h b/proxy/http/Http1ClientSession.h index 0cbe77a43fa..30113d5ad02 100644 --- a/proxy/http/Http1ClientSession.h +++ b/proxy/http/Http1ClientSession.h @@ -153,6 +153,12 @@ class Http1ClientSession : public ProxyClientSession client_vc->cancel_inactivity_timeout(); } + virtual const char * + get_protocol_string() const + { + return "http"; + } + private: Http1ClientSession(Http1ClientSession &); diff --git a/proxy/http/Http1ClientTransaction.h b/proxy/http/Http1ClientTransaction.h index ed2596f7869..9826b7fa5cd 100644 --- a/proxy/http/Http1ClientTransaction.h +++ b/proxy/http/Http1ClientTransaction.h @@ -94,11 +94,6 @@ class Http1ClientTransaction : public ProxyClientTransaction { return true; } - virtual const char * - get_protocol_string() const - { - return "http"; - } void set_parent(ProxyClientSession *new_parent); diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 7d192d5cf96..12c5da84d2f 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -311,6 +311,7 @@ HttpSM::HttpSM() client_tcp_reused(false), client_ssl_reused(false), client_connection_is_ssl(false), + client_protocol("-"), client_sec_protocol("-"), client_cipher_suite("-"), server_transact_count(0), @@ -536,6 +537,8 @@ HttpSM::attach_client_session(ProxyClientTransaction *client_vc, IOBufferReader const char *cipher = ssl_vc->getSSLCipherSuite(); client_cipher_suite = cipher ? cipher : "-"; } + const char *protocol_str = client_vc->get_protocol_string(); + client_protocol = protocol_str ? protocol_str : "-"; ink_release_assert(ua_session->get_half_close_flag() == false); mutex = client_vc->mutex; diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h index 94508824359..4379757c532 100644 --- a/proxy/http/HttpSM.h +++ b/proxy/http/HttpSM.h @@ -494,6 +494,7 @@ class HttpSM : public Continuation // Info about client's SSL connection. bool client_ssl_reused; bool client_connection_is_ssl; + const char *client_protocol; const char *client_sec_protocol; const char *client_cipher_suite; int server_transact_count; diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc index 88d226f86fd..ca4cc889f12 100644 --- a/proxy/http2/Http2ClientSession.cc +++ b/proxy/http2/Http2ClientSession.cc @@ -434,15 +434,3 @@ Http2ClientSession::state_complete_frame_read(int event, void *edata) vio->reenable(); return 0; } - -int64_t -Http2ClientSession::getPluginId() const -{ - return con_id; -} - -char const * -Http2ClientSession::getPluginTag() const -{ - return "http/2"; -} diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h index 9751e1186a3..088f87e0513 100644 --- a/proxy/http2/Http2ClientSession.h +++ b/proxy/http2/Http2ClientSession.h @@ -148,7 +148,7 @@ class Http2Frame IOBufferReader *ioreader; }; -class Http2ClientSession : public ProxyClientSession, public PluginIdentity +class Http2ClientSession : public ProxyClientSession { public: typedef ProxyClientSession super; ///< Parent type. @@ -197,9 +197,6 @@ class Http2ClientSession : public ProxyClientSession, public PluginIdentity return upgrade_context; } - virtual char const *getPluginTag() const; - virtual int64_t getPluginId() const; - virtual int get_transact_count() const { @@ -222,6 +219,12 @@ class Http2ClientSession : public ProxyClientSession, public PluginIdentity return dying_event; } + virtual const char * + get_protocol_string() const + { + return "http/2"; + } + private: Http2ClientSession(Http2ClientSession &); // noncopyable Http2ClientSession &operator=(const Http2ClientSession &); // noncopyable diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index d9461a9ac5f..a5a0f7a505d 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -225,12 +225,6 @@ class Http2Stream : public ProxyClientTransaction return false; } - virtual const char * - get_protocol_string() const - { - return "http/2"; - } - virtual void set_active_timeout(ink_hrtime timeout_in); virtual void set_inactivity_timeout(ink_hrtime timeout_in); virtual void cancel_inactivity_timeout(); diff --git a/proxy/logging/LogAccessHttp.cc b/proxy/logging/LogAccessHttp.cc index fbb19c5b8c3..71561e33eca 100644 --- a/proxy/logging/LogAccessHttp.cc +++ b/proxy/logging/LogAccessHttp.cc @@ -619,31 +619,29 @@ LogAccessHttp::marshal_client_req_http_version(char *buf) int LogAccessHttp::marshal_client_req_protocol_version(char *buf) { - int len = INK_MIN_ALIGN; - char const *tag = m_http_sm->plugin_tag; + const char *protocol_str = m_http_sm->client_protocol; + int len = LogAccess::strlen(protocol_str); - if (!tag) { + // Set major & minor versions when protocol_str is not "http/2". + if (::strlen(protocol_str) == 4 && strncmp("http", protocol_str, 4) == 0) { if (m_client_request) { HTTPVersion versionObject = m_client_request->version_get(); int64_t major = HTTP_MAJOR(versionObject.m_version); int64_t minor = HTTP_MINOR(versionObject.m_version); if (major == 1 && minor == 1) { - tag = "http/1.1"; + protocol_str = "http/1.1"; } else if (major == 1 && minor == 0) { - tag = "http/1.0"; - } else if (major == 0 && minor == 9) { - tag = "http/0.9"; + protocol_str = "http/1.0"; } // else invalid http version - len = LogAccess::strlen(tag); } else { - tag = "*"; + protocol_str = "*"; } - } else { - len = LogAccess::strlen(tag); + + len = LogAccess::strlen(protocol_str); } if (buf) { - marshal_str(buf, tag, len); + marshal_str(buf, protocol_str, len); } return len;