From eaa6daae13dd5268b6af04f99e6ad603a740905f Mon Sep 17 00:00:00 2001 From: Persia Aziz Date: Tue, 3 Jan 2017 16:19:19 -0600 Subject: [PATCH] TS-2888: remove vararg and format parameters from build_error_resoponse --- proxy/http/HttpBodyFactory.cc | 14 +---- proxy/http/HttpBodyFactory.h | 24 ++++---- proxy/http/HttpSM.cc | 6 +- proxy/http/HttpTransact.cc | 108 ++++++++++++++++------------------ proxy/http/HttpTransact.h | 4 +- 5 files changed, 72 insertions(+), 84 deletions(-) diff --git a/proxy/http/HttpBodyFactory.cc b/proxy/http/HttpBodyFactory.cc index 313caf7ee8e..12285eeea83 100644 --- a/proxy/http/HttpBodyFactory.cc +++ b/proxy/http/HttpBodyFactory.cc @@ -63,7 +63,7 @@ char * HttpBodyFactory::fabricate_with_old_api(const char *type, HttpTransact::State *context, int64_t max_buffer_length, int64_t *resulting_buffer_length, char *content_language_out_buf, size_t content_language_buf_size, char *content_type_out_buf, size_t content_type_buf_size, - const char *format, va_list ap) + int format_size, const char *format) { char *buffer = nullptr; const char *lang_ptr = nullptr; @@ -124,16 +124,8 @@ HttpBodyFactory::fabricate_with_old_api(const char *type, HttpTransact::State *c /////////////////////////////////////////// // check if we don't need to format body // /////////////////////////////////////////// - if (format) { - // The length from ink_bvsprintf includes the trailing NUL, so adjust the final - // length accordingly. - int l = ink_bvsprintf(nullptr, format, ap); - if (l <= max_buffer_length) { - buffer = (char *)ats_malloc(l); - *resulting_buffer_length = ink_bvsprintf(buffer, format, ap) - 1; - plain_flag = true; - } - } + + buffer = (format == nullptr) ? nullptr : ats_strndup(format, format_size); ///////////////////////////////////////////////////////// // try to fabricate the desired type of error response // ///////////////////////////////////////////////////////// diff --git a/proxy/http/HttpBodyFactory.h b/proxy/http/HttpBodyFactory.h index b3f18a64822..126417a8de8 100644 --- a/proxy/http/HttpBodyFactory.h +++ b/proxy/http/HttpBodyFactory.h @@ -64,6 +64,7 @@ #include "HttpTransact.h" #include "Main.h" #include "ts/RawHashTable.h" +#include "ts/ink_sprintf.h" #define HTTP_BODY_TEMPLATE_MAGIC 0xB0DFAC00 #define HTTP_BODY_SET_MAGIC 0xB0DFAC55 @@ -156,21 +157,22 @@ class HttpBodyFactory /////////////////////// char *fabricate_with_old_api(const char *type, HttpTransact::State *context, int64_t max_buffer_length, int64_t *resulting_buffer_length, char *content_language_out_buf, size_t content_language_buf_size, - char *content_type_out_buf, size_t content_type_buf_size, const char *format, va_list ap); + char *content_type_out_buf, size_t content_type_buf_size, int format_size, const char *format); char * - fabricate_with_old_api_build_va(const char *type, HttpTransact::State *context, int64_t max_buffer_length, - int64_t *resulting_buffer_length, char *content_language_out_buf, - size_t content_language_buf_size, char *content_type_out_buf, size_t content_type_buf_size, - const char *format, ...) + getFormat(int64_t max_buffer_length, int64_t *resulting_buffer_length, const char *format, ...) { - char *msg; + char *msg = nullptr; va_list ap; - - va_start(ap, format); - msg = fabricate_with_old_api(type, context, max_buffer_length, resulting_buffer_length, content_language_out_buf, - content_language_buf_size, content_type_out_buf, content_type_buf_size, format, ap); - va_end(ap); + if (format) { + // The length from ink_bvsprintf includes the trailing NUL, so adjust the final + // length accordingly. + int l = ink_bvsprintf(nullptr, format, ap); + if (l <= max_buffer_length) { + msg = (char *)ats_malloc(l); + *resulting_buffer_length = ink_bvsprintf(msg, format, ap) - 1; + } + } return msg; } diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 09d7fed6bef..a70facff742 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -3496,12 +3496,10 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p) switch (event) { case VC_EVENT_INACTIVITY_TIMEOUT: - HttpTransact::build_error_response(&t_state, HTTP_STATUS_REQUEST_TIMEOUT, "POST Request timeout", "timeout#inactivity", - nullptr); + HttpTransact::build_error_response(&t_state, HTTP_STATUS_REQUEST_TIMEOUT, "POST Request timeout", "timeout#inactivity"); break; case VC_EVENT_ACTIVE_TIMEOUT: - HttpTransact::build_error_response(&t_state, HTTP_STATUS_REQUEST_TIMEOUT, "POST Request timeout", "timeout#activity", - nullptr); + HttpTransact::build_error_response(&t_state, HTTP_STATUS_REQUEST_TIMEOUT, "POST Request timeout", "timeout#activity"); break; } diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index a46f084ec8e..cd9d5dec7c5 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -565,7 +565,7 @@ HttpTransact::BadRequest(State *s) DebugTxn("http_trans", "[BadRequest]" "parser marked request bad"); bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid HTTP Request", "request#syntax_error", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid HTTP Request", "request#syntax_error"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -575,7 +575,7 @@ HttpTransact::Forbidden(State *s) DebugTxn("http_trans", "[Forbidden]" "IpAllow marked request forbidden"); bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); - build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied", NULL); + build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, NULL); } @@ -642,7 +642,7 @@ HttpTransact::HandleBlindTunnel(State *s) // The error message we send back will be suppressed so // the only important thing in selecting the error is what // status code it gets logged as - build_error_response(s, HTTP_STATUS_INTERNAL_SERVER_ERROR, "Port Forwarding Error", "default", nullptr); + build_error_response(s, HTTP_STATUS_INTERNAL_SERVER_ERROR, "Port Forwarding Error", "default"); int host_len; const char *host = s->hdr_info.client_request.url_get()->host_get(&host_len); @@ -757,9 +757,9 @@ HttpTransact::EndRemapRequest(State *s) if (s->remap_redirect != nullptr) { SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); if (s->http_return_code == HTTP_STATUS_MOVED_PERMANENTLY) { - build_error_response(s, HTTP_STATUS_MOVED_PERMANENTLY, "Redirect", "redirect#moved_permanently", nullptr); + build_error_response(s, HTTP_STATUS_MOVED_PERMANENTLY, "Redirect", "redirect#moved_permanently"); } else { - build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect", "redirect#moved_temporarily", nullptr); + build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect", "redirect#moved_temporarily"); } ats_free(s->remap_redirect); s->reverse_proxy = false; @@ -773,7 +773,7 @@ HttpTransact::EndRemapRequest(State *s) // We must close this connection if client_connection_enabled == false // ///////////////////////////////////////////////////////////////////////// if (!s->client_connection_enabled) { - build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied", nullptr); + build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied"); s->reverse_proxy = false; goto done; } @@ -781,7 +781,7 @@ HttpTransact::EndRemapRequest(State *s) // Check if remap plugin set HTTP return code and return body // ///////////////////////////////////////////////////////////////// if (s->http_return_code != HTTP_STATUS_NONE) { - build_error_response(s, s->http_return_code, nullptr, nullptr, s->internal_msg_buffer_size ? s->internal_msg_buffer : nullptr); + build_error_response(s, s->http_return_code, nullptr, nullptr); s->reverse_proxy = false; goto done; } @@ -825,14 +825,14 @@ HttpTransact::EndRemapRequest(State *s) SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); if (redirect_url) { /* there is a redirect url */ - build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect For Explanation", "request#no_host", nullptr); + build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect For Explanation", "request#no_host"); s->hdr_info.client_response.value_set(MIME_FIELD_LOCATION, MIME_LEN_LOCATION, redirect_url, redirect_url_len); // socket when there is no host. Need to handle DNS failure elsewhere. } else if (host == nullptr) { /* no host */ - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Header Required", "request#no_host", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Header Required", "request#no_host"); s->squid_codes.log_code = SQUID_LOG_ERR_INVALID_URL; } else { - build_error_response(s, HTTP_STATUS_NOT_FOUND, "Not Found on Accelerator", "urlrouting#no_mapping", nullptr); + build_error_response(s, HTTP_STATUS_NOT_FOUND, "Not Found on Accelerator", "urlrouting#no_mapping"); s->squid_codes.log_code = SQUID_LOG_ERR_INVALID_URL; } s->reverse_proxy = false; @@ -845,7 +845,7 @@ HttpTransact::EndRemapRequest(State *s) /////////////////////////////////////////////////////// SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_NOT_FOUND, "Not Found", "urlrouting#no_mapping", nullptr); + build_error_response(s, HTTP_STATUS_NOT_FOUND, "Not Found", "urlrouting#no_mapping"); s->squid_codes.log_code = SQUID_LOG_ERR_INVALID_URL; s->reverse_proxy = false; @@ -979,7 +979,7 @@ HttpTransact::handle_upgrade_request(State *s) DebugTxn("http_trans_upgrade", "Transaction requested upgrade for unknown protocol: %s", upgrade_hdr_val); } - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Upgrade Request", "request#syntax_error", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Upgrade Request", "request#syntax_error"); // we want our modify_request method to just return while we fail out from here. // this seems like the preferred option because the user wanted to do an upgrade but sent a bad protocol. @@ -1008,7 +1008,7 @@ HttpTransact::handle_websocket_upgrade_pre_remap(State *s) url->scheme_set(URL_SCHEME_WSS, URL_LEN_WSS); } else { DebugTxn("http_trans_websocket_upgrade_pre_remap", "Invalid scheme for websocket upgrade"); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Upgrade Request", "request#syntax_error", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Upgrade Request", "request#syntax_error"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -1168,12 +1168,12 @@ HttpTransact::handleIfRedirect(State *s) redirect_url.destroy(); if (answer == TEMPORARY_REDIRECT) { if ((s->client_info).http_version.m_version == HTTP_VERSION(1, 1)) { - build_error_response(s, HTTP_STATUS_TEMPORARY_REDIRECT, "Redirect", "redirect#moved_temporarily", nullptr); + build_error_response(s, HTTP_STATUS_TEMPORARY_REDIRECT, "Redirect", "redirect#moved_temporarily"); } else { - build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect", "redirect#moved_temporarily", nullptr); + build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect", "redirect#moved_temporarily"); } } else { - build_error_response(s, HTTP_STATUS_MOVED_PERMANENTLY, "Redirect", "redirect#moved_permanently", nullptr); + build_error_response(s, HTTP_STATUS_MOVED_PERMANENTLY, "Redirect", "redirect#moved_permanently"); } s->arena.str_free(s->remap_redirect); s->remap_redirect = nullptr; @@ -1226,7 +1226,7 @@ HttpTransact::HandleRequest(State *s) s->is_websocket = false; // unset to avoid screwing up stats. DebugTxn("http_trans", "Rejecting websocket connection because the limit has been exceeded"); bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); - build_error_response(s, HTTP_STATUS_SERVICE_UNAVAILABLE, "WebSocket Connection Limit Exceeded", nullptr, nullptr); + build_error_response(s, HTTP_STATUS_SERVICE_UNAVAILABLE, "WebSocket Connection Limit Exceeded", nullptr); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } } @@ -1238,7 +1238,7 @@ HttpTransact::HandleRequest(State *s) s->http_config_param->max_post_size); HTTP_INCREMENT_DYN_STAT(http_post_body_too_large); bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); - build_error_response(s, HTTP_STATUS_REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large", "request#entity_too_large", nullptr); + build_error_response(s, HTTP_STATUS_REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large", "request#entity_too_large"); s->squid_codes.log_code = SQUID_LOG_ERR_POST_ENTITY_TOO_LARGE; TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -1255,7 +1255,7 @@ HttpTransact::HandleRequest(State *s) // Let's error out this request. DebugTxn("http_trans", "Client sent a post expect: 100-continue, sending 405."); HTTP_INCREMENT_DYN_STAT(disallowed_post_100_continue); - build_error_response(s, HTTP_STATUS_METHOD_NOT_ALLOWED, "Method Not Allowed", "request#method_unsupported", nullptr); + build_error_response(s, HTTP_STATUS_METHOD_NOT_ALLOWED, "Method Not Allowed", "request#method_unsupported"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } } @@ -1335,7 +1335,7 @@ HttpTransact::HandleRequest(State *s) StartAccessControl(s); return; } else if (s->http_config_param->no_origin_server_dns) { - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Next Hop Connection Failed", "connect#failed_connect", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Next Hop Connection Failed", "connect#failed_connect"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -1575,7 +1575,7 @@ HttpTransact::ReDNSRoundRobin(State *s) s->next_action = how_to_open_connection(s); } else { // Our ReDNS failed so output the DNS failure error message - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Cannot find server.", "connect#dns_failed", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Cannot find server.", "connect#dns_failed"); s->cache_info.action = CACHE_DO_NO_ACTION; s->next_action = SM_ACTION_SEND_ERROR_CACHE_NOOP; // s->next_action = PROXY_INTERNAL_CACHE_NOOP; @@ -1667,7 +1667,7 @@ HttpTransact::OSDNSLookup(State *s) } // output the DNS failure error message SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Cannot find server.", "connect#dns_failed", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Cannot find server.", "connect#dns_failed"); // s->cache_info.action = CACHE_DO_NO_ACTION; TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -1791,7 +1791,7 @@ HttpTransact::OSDNSLookup(State *s) TRANSACT_RETURN(SM_ACTION_API_OS_DNS, HandleCacheOpenReadMiss); // DNS lookup is done if the lookup failed and need to call Handle Cache Open Read Miss } else { - build_error_response(s, HTTP_STATUS_INTERNAL_SERVER_ERROR, "Invalid Cache Lookup result", "default", nullptr); + build_error_response(s, HTTP_STATUS_INTERNAL_SERVER_ERROR, "Invalid Cache Lookup result", "default"); Log::error("HTTP: Invalid CACHE LOOKUP RESULT : %d", s->cache_lookup_result); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -1845,7 +1845,7 @@ HttpTransact::HandleFiltering(State *s) SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); // adding a comment so that cvs recognizes that I added a space in the text below - build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied", nullptr); + build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied"); // s->cache_info.action = CACHE_DO_NO_ACTION; TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -2123,7 +2123,7 @@ HttpTransact::HandlePushError(State *s, const char *reason) // reset from the body still being transfered s->state_machine->set_ua_half_close_flag(); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, reason, "default", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, reason, "default"); } /////////////////////////////////////////////////////////////////////////////// @@ -2859,7 +2859,7 @@ HttpTransact::build_response_from_cache(State *s, HTTPWarningCode warning_code) if (client_response_code == HTTP_STATUS_OK && client_request->presence(MIME_PRESENCE_RANGE)) { s->state_machine->do_range_setup_if_necessary(); if (s->range_setup == RANGE_NOT_SATISFIABLE) { - build_error_response(s, HTTP_STATUS_RANGE_NOT_SATISFIABLE, "Requested Range Not Satisfiable", "default", nullptr); + build_error_response(s, HTTP_STATUS_RANGE_NOT_SATISFIABLE, "Requested Range Not Satisfiable", "default"); s->cache_info.action = CACHE_DO_NO_ACTION; s->next_action = SM_ACTION_INTERNAL_CACHE_NOOP; break; @@ -2900,7 +2900,7 @@ HttpTransact::build_response_from_cache(State *s, HTTPWarningCode warning_code) // and server is not reacheable: 502 // DebugTxn("http_trans", "[build_response_from_cache] No match! Connection failed."); - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Connection Failed", "connect#failed_connect", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Connection Failed", "connect#failed_connect"); s->cache_info.action = CACHE_DO_NO_ACTION; s->next_action = SM_ACTION_INTERNAL_CACHE_NOOP; warning_code = HTTP_WARNING_CODE_NONE; @@ -2950,7 +2950,7 @@ HttpTransact::handle_cache_write_lock(State *s) case CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE: DebugTxn("http_error", "cache_open_write_fail_action %d, cache miss, return error", s->cache_open_write_fail_action); s->cache_info.write_status = CACHE_WRITE_ERROR; - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Connection Failed", "connect#failed_connect", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Connection Failed", "connect#failed_connect"); MIMEField *ats_field; HTTPHdr *header; header = &(s->hdr_info.client_response); @@ -3137,7 +3137,7 @@ HttpTransact::HandleCacheOpenReadMiss(State *s) s->next_action = how_to_open_connection(s); } else { // miss, but only-if-cached is set - build_error_response(s, HTTP_STATUS_GATEWAY_TIMEOUT, "Not Cached", "cache#not_in_cache", nullptr); + build_error_response(s, HTTP_STATUS_GATEWAY_TIMEOUT, "Not Cached", "cache#not_in_cache"); s->next_action = SM_ACTION_SEND_ERROR_CACHE_NOOP; } @@ -4302,7 +4302,7 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) /* Downgrade the request level and retry */ if (!HttpTransactHeaders::downgrade_request(&keep_alive, &s->hdr_info.server_request)) { - build_error_response(s, HTTP_STATUS_HTTPVER_NOT_SUPPORTED, "HTTP Version Not Supported", "response#bad_version", nullptr); + build_error_response(s, HTTP_STATUS_HTTPVER_NOT_SUPPORTED, "HTTP Version Not Supported", "response#bad_version"); s->next_action = SM_ACTION_SEND_ERROR_CACHE_NOOP; s->already_downgraded = true; } else { @@ -4666,7 +4666,7 @@ HttpTransact::handle_no_cache_operation_on_forward_server_response(State *s) /* Downgrade the request level and retry */ if (!HttpTransactHeaders::downgrade_request(&keep_alive, &s->hdr_info.server_request)) { s->already_downgraded = true; - build_error_response(s, HTTP_STATUS_HTTPVER_NOT_SUPPORTED, "HTTP Version Not Supported", "response#bad_version", nullptr); + build_error_response(s, HTTP_STATUS_HTTPVER_NOT_SUPPORTED, "HTTP Version Not Supported", "response#bad_version"); s->next_action = SM_ACTION_SEND_ERROR_CACHE_NOOP; } else { s->already_downgraded = true; @@ -6387,14 +6387,14 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request) DebugTxn("http_trans", "[is_request_valid] failed proxy authorization"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); build_error_response(s, HTTP_STATUS_PROXY_AUTHENTICATION_REQUIRED, "Proxy Authentication Required", - "access#proxy_auth_required", nullptr); + "access#proxy_auth_required"); return false; case NON_EXISTANT_REQUEST_HEADER: /* fall through */ case BAD_HTTP_HEADER_SYNTAX: { DebugTxn("http_trans", "[is_request_valid] non-existant/bad header"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid HTTP Request", "request#syntax_error", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid HTTP Request", "request#syntax_error"); return false; } @@ -6418,10 +6418,10 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request) DebugTxn("http_trans", "[is_request_valid] missing host field"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); if (s->http_config_param->reverse_proxy_enabled) { // host header missing and reverse proxy on - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Header Required", "request#no_host", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Header Required", "request#no_host"); } else { // host header missing and reverse proxy off - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Required In Request", "request#no_host", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Host Required In Request", "request#no_host"); } return false; @@ -6429,7 +6429,7 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request) case NO_REQUEST_SCHEME: { DebugTxn("http_trans", "[is_request_valid] unsupported or missing request scheme"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Unsupported URL Scheme", "request#scheme_unsupported", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Unsupported URL Scheme", "request#scheme_unsupported"); return false; } /* fall through */ @@ -6442,24 +6442,24 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request) port = url ? url->port_get() : 0; DebugTxn("http_trans", "[is_request_valid] %d is an invalid connect port", port); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_FORBIDDEN, "Tunnel Forbidden", "access#connect_forbidden", nullptr); + build_error_response(s, HTTP_STATUS_FORBIDDEN, "Tunnel Forbidden", "access#connect_forbidden"); return false; case NO_POST_CONTENT_LENGTH: { DebugTxn("http_trans", "[is_request_valid] post request without content length"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_LENGTH_REQUIRED, "Content Length Required", "request#no_content_length", nullptr); + build_error_response(s, HTTP_STATUS_LENGTH_REQUIRED, "Content Length Required", "request#no_content_length"); return false; } case UNACCEPTABLE_TE_REQUIRED: { DebugTxn("http_trans", "[is_request_valid] TE required is unacceptable."); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_NOT_ACCEPTABLE, "Transcoding Not Available", "transcoding#unsupported", nullptr); + build_error_response(s, HTTP_STATUS_NOT_ACCEPTABLE, "Transcoding Not Available", "transcoding#unsupported"); return false; } case INVALID_POST_CONTENT_LENGTH: { DebugTxn("http_trans", "[is_request_valid] post request with negative content length value"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Content Length", "request#invalid_content_length", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Content Length", "request#invalid_content_length"); return false; } default: @@ -6678,7 +6678,7 @@ HttpTransact::will_this_request_self_loop(State *s) "unknown's ip and port same as local ip and port - bailing"); break; } - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Cycle Detected", "request#cycle_detected", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Cycle Detected", "request#cycle_detected"); return true; } } @@ -6696,7 +6696,7 @@ HttpTransact::will_this_request_self_loop(State *s) if (via_string && ptr_len_str(via_string, via_len, uuid)) { DebugTxn("http_transact", "[will_this_request_self_loop] Incoming via: %.*s has (%s[%s] (%s))", via_len, via_string, s->http_config_param->proxy_hostname, uuid, s->http_config_param->proxy_request_via_string); - build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Multi-Hop Cycle Detected", "request#cycle_detected", nullptr); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Multi-Hop Cycle Detected", "request#cycle_detected"); return true; } @@ -7600,7 +7600,7 @@ HttpTransact::handle_parent_died(State *s) { ink_assert(s->parent_result.result == PARENT_FAIL); - build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Next Hop Connection Failed", "connect#failed_connect", nullptr); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Next Hop Connection Failed", "connect#failed_connect"); TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } @@ -7742,7 +7742,7 @@ HttpTransact::handle_server_died(State *s) body_type = "connect#failed_connect"; } - build_error_response(s, status, reason, body_type, nullptr); + build_error_response(s, status, reason, body_type); return; } @@ -8069,10 +8069,8 @@ HttpTransact::build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing // ////////////////////////////////////////////////////////////////////////////// void -HttpTransact::build_error_response(State *s, HTTPStatus status_code, const char *reason_phrase_or_null, const char *error_body_type, - const char *format, ...) +HttpTransact::build_error_response(State *s, HTTPStatus status_code, const char *reason_phrase_or_null, const char *error_body_type) { - va_list ap; const char *reason_phrase; char *url_string; char body_language[256], body_type[256]; @@ -8201,10 +8199,9 @@ HttpTransact::build_error_response(State *s, HTTPStatus status_code, const char int64_t len; char *new_msg; - va_start(ap, format); - new_msg = body_factory->fabricate_with_old_api(error_body_type, s, s->http_config_param->body_factory_response_max_size, &len, - body_language, sizeof(body_language), body_type, sizeof(body_type), format, ap); - va_end(ap); + new_msg = body_factory->fabricate_with_old_api(error_body_type, s, 8192, &len, body_language, sizeof(body_language), body_type, + sizeof(body_type), s->internal_msg_buffer_size, + s->internal_msg_buffer_size ? s->internal_msg_buffer : nullptr); // After the body factory is called, a new "body" is allocated, and we must replace it. It is // unfortunate that there's no way to avoid this fabrication even when there is no substitutions... @@ -8258,7 +8255,6 @@ HttpTransact::build_redirect_response(State *s) const char *new_url = nullptr; int new_url_len; char *to_free = nullptr; - char body_language[256], body_type[256]; HTTPStatus status_code = HTTP_STATUS_MOVED_TEMPORARILY; char *reason_phrase = (char *)(http_hdr_reason_lookup(status_code)); @@ -8296,10 +8292,10 @@ HttpTransact::build_redirect_response(State *s) ////////////////////////// s->free_internal_msg_buffer(); s->internal_msg_buffer_fast_allocator_size = -1; - s->internal_msg_buffer = body_factory->fabricate_with_old_api_build_va( - "redirect#moved_temporarily", s, 8192, &s->internal_msg_buffer_size, body_language, sizeof(body_language), body_type, - sizeof(body_type), "%s %s. %s.", "The document you requested is now", new_url, new_url, - "Please update your documents and bookmarks accordingly", NULL); + // template redirect#temporarily can not be used here since there is no way to pass the computed url to the template. + s->internal_msg_buffer = body_factory->getFormat(8192, &s->internal_msg_buffer_size, "%s %s. %s.", + "The document you requested is now", new_url, new_url, + "Please update your documents and bookmarks accordingly", NULL); h->set_content_length(s->internal_msg_buffer_size); h->value_set(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, "text/html", 9); diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index ed0b348b806..6d451dd0d1a 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -1317,8 +1317,8 @@ class HttpTransact HTTPHdr *obj_response); static void handle_parent_died(State *s); static void handle_server_died(State *s); - static void build_error_response(State *s, HTTPStatus status_code, const char *reason_phrase_or_null, const char *error_body_type, - const char *format, ...); + static void build_error_response(State *s, HTTPStatus status_code, const char *reason_phrase_or_null, + const char *error_body_type); static void build_redirect_response(State *s); static void build_upgrade_response(State *s); static const char *get_error_string(int erno);