TS-3140: Traffic Server asserts during response redirect.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/9e2689b7 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/9e2689b7 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/9e2689b7 Branch: refs/heads/master Commit: 9e2689b78a3ab6b768ec60506057b065206f4e0d Parents: 179bb98 Author: shinrich <shinr...@yahoo-inc.com> Authored: Thu Jan 22 08:15:21 2015 -0600 Committer: shinrich <shinr...@yahoo-inc.com> Committed: Thu Jan 22 08:15:21 2015 -0600 ---------------------------------------------------------------------- CHANGES | 1 + proxy/InkAPI.cc | 7 +++++-- proxy/http/HttpSM.cc | 24 ++++++++++++++++-------- proxy/http/HttpTransact.cc | 3 --- proxy/http/HttpTransact.h | 2 -- proxy/http/HttpTunnel.cc | 14 ++++++++++---- 6 files changed, 32 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 70c038b..ddbd7e0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,6 @@ -*- coding: utf-8 -*- Changes with Apache Traffic Server 5.3.0 + *) [TS-3140] Traffic Server asserts during response redirect. *) [TS-3309] Document TLS session ticket rotation. Author: Bin Zeng <bz...@linkedin.com> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/InkAPI.cc ---------------------------------------------------------------------- diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc index c59573e..7825eb7 100644 --- a/proxy/InkAPI.cc +++ b/proxy/InkAPI.cc @@ -4957,6 +4957,11 @@ TSHttpTxnSecondUrlTryLock(TSHttpTxn txnp) return TS_SUCCESS; } +/* + * TSHttpTxnRedirectRequest is very odd. It is only in experimental.h. + * It is not used in any checked in code. We should probably remove this. + * SKH 1/15/2015 + */ TSReturnCode TSHttpTxnRedirectRequest(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc) { @@ -4991,8 +4996,6 @@ TSHttpTxnRedirectRequest(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc) r_url->copy(&u); s->hdr_info.server_request.destroy(); - // we want to close the server session - s->api_release_server_session = true; s->request_sent_time = 0; s->response_received_time = 0; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpSM.cc ---------------------------------------------------------------------- diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 99da799..3998771 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -1515,13 +1515,9 @@ HttpSM::handle_api_return() case HttpTransact::SM_ACTION_API_READ_CACHE_HDR: case HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR: case HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE: - // this part is added for automatic redirect - if (t_state.api_next_action == HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR && t_state.api_release_server_session) { - t_state.api_release_server_session = false; - release_server_session(); - } else if (t_state.api_next_action == HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE && - t_state.api_cleanup_cache_read && - t_state.api_update_cached_object != HttpTransact::UPDATE_CACHED_OBJECT_PREPARE) { + if (t_state.api_next_action == HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE && + t_state.api_cleanup_cache_read && + t_state.api_update_cached_object != HttpTransact::UPDATE_CACHED_OBJECT_PREPARE) { t_state.api_cleanup_cache_read = false; t_state.cache_info.object_read = NULL; t_state.request_sent_time = UNDEFINED_TIME; @@ -1603,6 +1599,11 @@ HttpSM::handle_api_return() case HttpTransact::SM_ACTION_REDIRECT_READ: { + // Clean up from any communication with previous servers + release_server_session(); + cache_sm.close_write(); + //tunnel.deallocate_redirect_postdata_buffers(); + call_transact_and_set_next_state(HttpTransact::HandleRequest); break; } @@ -5059,6 +5060,7 @@ HttpSM::release_server_session(bool serve_from_cache) if (TS_SERVER_SESSION_SHARING_MATCH_NONE != t_state.txn_conf->server_session_sharing_match && t_state.current.server->keep_alive == HTTP_KEEPALIVE && t_state.hdr_info.server_response.valid() && + t_state.hdr_info.server_request.valid() && (t_state.hdr_info.server_response.status_get() == HTTP_STATUS_NOT_MODIFIED || (t_state.hdr_info.server_request.method_get_wksidx() == HTTP_WKSIDX_HEAD && t_state.www_auth_content != HttpTransact::CACHE_AUTH_NONE)) && @@ -7600,8 +7602,10 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len) valid_origHost = false; t_state.hdr_info.server_request.destroy(); + // we want to close the server session - t_state.api_release_server_session = true; + // will do that in handle_api_return under the + // HttpTransact::SM_ACTION_REDIRECT_READ state t_state.parent_result.r = PARENT_UNDEFINED; t_state.request_sent_time = 0; t_state.response_received_time = 0; @@ -7611,6 +7615,10 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len) t_state.dns_info.lookup_success = false; t_state.force_dns = false; + if (t_state.txn_conf->cache_http) { + t_state.cache_info.object_read = NULL; + } + bool noPortInHost = HttpConfig::m_master.redirection_host_no_port; // check to see if the client request passed a host header, if so copy the host and port from the redirect url and http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTransact.cc ---------------------------------------------------------------------- diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index c6f55ed..987a084 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -1242,9 +1242,6 @@ HttpTransact::HandleRequest(State* s) HTTP_INCREMENT_TRANS_STAT(https_incoming_requests_stat); } - if (s->api_release_server_session == true) { - s->api_release_server_session = false; - } /////////////////////////////////////////////// // if request is bad, return error response // /////////////////////////////////////////////// http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTransact.h ---------------------------------------------------------------------- diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index b2b6b42..f30f19c 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -969,7 +969,6 @@ public: // These ptrs are deallocate when transaction is over. HdrHeapSDKHandle *cache_req_hdr_heap_handle; HdrHeapSDKHandle *cache_resp_hdr_heap_handle; - bool api_release_server_session; bool api_cleanup_cache_read; bool api_server_response_no_store; bool api_server_response_ignore; @@ -1082,7 +1081,6 @@ public: api_txn_no_activity_timeout_value(-1), cache_req_hdr_heap_handle(NULL), cache_resp_hdr_heap_handle(NULL), - api_release_server_session(false), api_cleanup_cache_read(false), api_server_response_no_store(false), api_server_response_ignore(false), http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTunnel.cc ---------------------------------------------------------------------- diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index a043b63..d75b5a1 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -1638,11 +1638,17 @@ HttpTunnel::allocate_redirect_postdata_buffers(IOBufferReader * ua_reader) // If fixed, obviously also fix the deallocator. if (postbuf == NULL) { postbuf = new PostDataBuffers(); + postbuf->ua_buffer_reader = ua_reader; + postbuf->postdata_copy_buffer = new_MIOBuffer(alloc_index); + postbuf->postdata_copy_buffer_start = postbuf->postdata_copy_buffer->alloc_reader(); + allocate_redirect_postdata_producer_buffer(); + } else { + // Reset the buffer readers + postbuf->postdata_copy_buffer->dealloc_reader(postbuf->postdata_copy_buffer_start); + postbuf->postdata_copy_buffer_start = postbuf->postdata_copy_buffer->alloc_reader(); + postbuf->postdata_producer_buffer->dealloc_reader(postbuf->postdata_producer_reader); + postbuf->postdata_producer_reader = postbuf->postdata_producer_buffer->alloc_reader(); } - postbuf->ua_buffer_reader = ua_reader; - postbuf->postdata_copy_buffer = new_MIOBuffer(alloc_index); - postbuf->postdata_copy_buffer_start = postbuf->postdata_copy_buffer->alloc_reader(); - allocate_redirect_postdata_producer_buffer(); }