From cb7d71a6b1a0789fe66c424c51f137787a12e70b Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Thu, 6 Oct 2016 16:20:08 +0000 Subject: [PATCH] TS-4938: Avoid crashes due to NULL vc dereferences. One more null check. Remove server_session null checks. And address James' comments. clang-format (cherry picked from commit 784e7cc8a91b48b93b57cad27ba403684880969f) Conflicts: proxy/http/HttpSM.cc proxy/http/HttpServerSession.h --- proxy/http/HttpSM.cc | 23 +++++++---------------- proxy/http/HttpTransact.cc | 25 ++++++++++++------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index b7a1be2c3a9..bafd6fb2287 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -1632,8 +1632,8 @@ HttpSM::handle_api_return() DebugSM("http_websocket", "(client session) Setting websocket active timeout=%" PRId64 "s and inactive timeout=%" PRId64 "s", t_state.txn_conf->websocket_active_timeout, t_state.txn_conf->websocket_inactive_timeout); - ua_session->get_netvc()->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_active_timeout)); - ua_session->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_inactive_timeout)); + ua_session->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_active_timeout)); + ua_session->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_inactive_timeout)); } if (server_session) { @@ -5778,12 +5778,6 @@ HttpSM::attach_server_session(HttpServerSession *s) return; } - if (ua_session) { - NetVConnection *server_vc = s->get_netvc(); - NetVConnection *ua_vc = ua_session->get_netvc(); - ink_release_assert(server_vc->thread == ua_vc->thread); - } - // Set the mutex so that we have something to update // stats with server_session->mutex = this->mutex; @@ -5802,7 +5796,10 @@ HttpSM::attach_server_session(HttpServerSession *s) UnixNetVConnection *server_vc = dynamic_cast(server_session->get_netvc()); UnixNetVConnection *client_vc = (UnixNetVConnection *)(ua_session->get_netvc()); SSLNetVConnection *ssl_vc = dynamic_cast(client_vc); - bool associated_connection = false; + + // Verifying that the user agent and server sessions/transactions are operating on the same thread. + ink_release_assert(!server_vc || !client_vc || server_vc->thread == client_vc->thread); + bool associated_connection = false; if (server_vc) { // if server_vc isn't a PluginVC if (ssl_vc) { // if incoming connection is SSL bool client_trace = ssl_vc->getSSLTrace(); @@ -7251,13 +7248,7 @@ HttpSM::set_next_state() // sending its request and for this reason, the inactivity timeout // cannot be cancelled. if (ua_session && !t_state.hdr_info.request_content_length) { - NetVConnection *vc = ua_session->get_netvc(); - if (vc) { - ua_session->cancel_inactivity_timeout(); - } else { - terminate_sm = true; - return; // Give up if there is no session netvc - } + ua_session->cancel_inactivity_timeout(); } else if (!ua_session) { terminate_sm = true; return; // Give up if there is no session diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index a7df6517233..f8abad79f3b 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -558,7 +558,8 @@ HttpTransact::BadRequest(State *s) void HttpTransact::HandleBlindTunnel(State *s) { - bool inbound_transparent_p = s->state_machine->ua_session->get_netvc()->get_is_transparent(); + NetVConnection *vc = s->state_machine->ua_session->get_netvc(); + bool inbound_transparent_p = vc->get_is_transparent(); URL u; // IpEndpoint dest_addr; // ip_text_buffer new_host; @@ -580,10 +581,10 @@ HttpTransact::HandleBlindTunnel(State *s) s->hdr_info.client_request.version_set(ver); char new_host[INET6_ADDRSTRLEN]; - ats_ip_ntop(s->state_machine->ua_session->get_netvc()->get_local_addr(), new_host, sizeof(new_host)); + ats_ip_ntop(vc->get_local_addr(), new_host, sizeof(new_host)); s->hdr_info.client_request.url_get()->host_set(new_host, strlen(new_host)); - s->hdr_info.client_request.url_get()->port_set(s->state_machine->ua_session->get_netvc()->get_local_port()); + s->hdr_info.client_request.url_get()->port_set(vc->get_local_port()); // Initialize the state vars necessary to sending error responses bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); @@ -918,7 +919,7 @@ HttpTransact::EndRemapRequest(State *s) done: // We now set the active-timeout again, since it might have been changed as part of the remap rules. if (s->state_machine->ua_session) { - s->state_machine->ua_session->get_netvc()->set_active_timeout(HRTIME_SECONDS(s->txn_conf->transaction_active_timeout_in)); + s->state_machine->ua_session->set_active_timeout(HRTIME_SECONDS(s->txn_conf->transaction_active_timeout_in)); } if (is_debug_tag_set("http_chdr_describe") || is_debug_tag_set("http_trans") || is_debug_tag_set("url_rewrite")) { @@ -5643,14 +5644,15 @@ HttpTransact::initialize_state_variables_from_request(State *s, HTTPHdr *obsolet s->client_info.proxy_connect_hdr = true; } - if (s->state_machine->ua_session) { - s->request_data.incoming_port = s->state_machine->ua_session->get_netvc()->get_local_port(); - s->request_data.internal_txn = s->state_machine->ua_session->get_netvc()->get_is_internal_request(); - } NetVConnection *vc = NULL; if (s->state_machine->ua_session) { vc = s->state_machine->ua_session->get_netvc(); } + + if (vc) { + s->request_data.incoming_port = vc->get_local_port(); + s->request_data.internal_txn = vc->get_is_internal_request(); + } // If this is an internal request, never keep alive if (!s->txn_conf->keep_alive_enabled_in || (vc && vc->get_is_internal_request()) || (s->state_machine->ua_session && s->state_machine->ua_session->ignore_keep_alive())) { @@ -5733,11 +5735,8 @@ HttpTransact::initialize_state_variables_from_request(State *s, HTTPHdr *obsolet s->request_data.hostname_str = s->arena.str_store(host_name, host_len); ats_ip_copy(&s->request_data.src_ip, &s->client_info.src_addr); memset(&s->request_data.dest_ip, 0, sizeof(s->request_data.dest_ip)); - if (s->state_machine->ua_session) { - NetVConnection *netvc = s->state_machine->ua_session->get_netvc(); - if (netvc) { - s->request_data.incoming_port = netvc->get_local_port(); - } + if (vc) { + s->request_data.incoming_port = vc->get_local_port(); } s->request_data.xact_start = s->client_request_time; s->request_data.api_info = &s->api_info;