You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2021/11/08 23:14:39 UTC

[trafficserver] branch 9.2.x updated: Make factory.response_suppression_mode an overridable config (#8469)

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/9.2.x by this push:
     new eb719a2  Make factory.response_suppression_mode an overridable config (#8469)
eb719a2 is described below

commit eb719a285319c8557d70d99da2442211cfb57d77
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Fri Oct 29 21:46:04 2021 -0600

    Make factory.response_suppression_mode an overridable config (#8469)
    
    * Make factory.response_suppression_mode an overridable config
    
    This changes proxy.config.body_factory.response_suppression_mode to be
    an overridable configuration in the Http "state". This can be used to avoid
    having body factory populating a body when it's undeseriable.
    
    A typical use case would be e.g. this configuration for HRW:
    
    cond %{REMAP_PSEUDO_HOOK}
        set-redirect 302 http://www.example.com
        set-config proxy.config.body_factory.response_suppression_mode 1
    
    * Fixes a core issue related to the BodyFactory suppression
    
    Author: Sudheer V.
    Review: Leif
    
    Co-authored-by: Sudheer Vinukonda <su...@apache.org>
    (cherry picked from commit cf7849270c676b36a15ae4fe367d00e6b274f833)
---
 doc/admin-guide/files/records.config.en.rst        |  2 ++
 doc/admin-guide/plugins/lua.en.rst                 |  1 +
 .../api/functions/TSHttpOverridableConfig.en.rst   |  1 +
 .../api/types/TSOverridableConfigKey.en.rst        |  1 +
 include/ts/apidefs.h.in                            |  1 +
 plugins/lua/ts_lua_http_config.c                   |  2 ++
 proxy/http/HttpBodyFactory.cc                      | 27 +++++++++-------------
 proxy/http/HttpBodyFactory.h                       |  8 +++----
 proxy/http/HttpConfig.cc                           |  5 ++++
 proxy/http/HttpConfig.h                            |  5 ++++
 proxy/http/HttpSM.cc                               |  6 ++++-
 src/shared/overridable_txn_vars.cc                 |  4 +++-
 src/traffic_server/InkAPI.cc                       |  4 ++++
 src/traffic_server/InkAPITest.cc                   |  3 ++-
 14 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index ffdba4d..437c99d 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -2569,6 +2569,8 @@ Customizable User Response Pages
     Maximum size of the error template response page.
 
 .. ts:cv:: CONFIG proxy.config.body_factory.response_suppression_mode INT 0
+    :reloadable:
+    :overridable:
 
    Specifies when |TS| suppresses generated response pages:
 
diff --git a/doc/admin-guide/plugins/lua.en.rst b/doc/admin-guide/plugins/lua.en.rst
index c6e505b..8da1cde 100644
--- a/doc/admin-guide/plugins/lua.en.rst
+++ b/doc/admin-guide/plugins/lua.en.rst
@@ -4008,6 +4008,7 @@ Http config constants
     TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX
     TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK
     TS_LUA_CONFIG_NET_SOCK_NOTSENT_LOWAT
+    TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE
     TS_LUA_CONFIG_LAST_ENTRY
 
 :ref:`TOP <admin-plugins-ts-lua>`
diff --git a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst
index d6bc066..94a9583 100644
--- a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst
+++ b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst
@@ -188,6 +188,7 @@ TSOverridableConfigKey Value                                              Config
 :c:enumerator:`TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX`                  :ts:cv:`proxy.config.plugin.vc.default_buffer_index`
 :c:enumerator:`TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK`             :ts:cv:`proxy.config.plugin.vc.default_buffer_water_mark`
 :c:enumerator:`TS_CONFIG_NET_SOCK_NOTSENT_LOWAT`                          :ts:cv:`proxy.config.net.sock_notsent_lowat`
+:c:enumerator:`TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE`          :ts:cv:`proxy.config.body_factory.response_suppression_mode`
 ========================================================================  ====================================================================
 
 Examples
diff --git a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst
index 50b9d8e..30000da 100644
--- a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst
+++ b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst
@@ -153,6 +153,7 @@ Enumeration Members
 .. c:enumerator:: TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX
 .. c:enumerator:: TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK
 .. c:enumerator:: TS_CONFIG_NET_SOCK_NOTSENT_LOWAT
+.. c:enumerator:: TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE
 
 
 Description
diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in
index be12d43..c7c8a18 100644
--- a/include/ts/apidefs.h.in
+++ b/include/ts/apidefs.h.in
@@ -866,6 +866,7 @@ typedef enum {
   TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX,
   TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK,
   TS_CONFIG_NET_SOCK_NOTSENT_LOWAT,
+  TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE,
   TS_CONFIG_LAST_ENTRY
 } TSOverridableConfigKey;
 
diff --git a/plugins/lua/ts_lua_http_config.c b/plugins/lua/ts_lua_http_config.c
index faa70ef..3f47762 100644
--- a/plugins/lua/ts_lua_http_config.c
+++ b/plugins/lua/ts_lua_http_config.c
@@ -142,6 +142,7 @@ typedef enum {
   TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX                = TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX,
   TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK           = TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK,
   TS_LUA_CONFIG_NET_SOCK_NOTSENT_LOWAT                        = TS_CONFIG_NET_SOCK_NOTSENT_LOWAT,
+  TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE        = TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE,
   TS_LUA_CONFIG_LAST_ENTRY                                    = TS_CONFIG_LAST_ENTRY,
 } TSLuaOverridableConfigKey;
 
@@ -276,6 +277,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = {
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_NET_SOCK_NOTSENT_LOWAT),
+  TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE),
   TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_LAST_ENTRY),
 };
 
diff --git a/proxy/http/HttpBodyFactory.cc b/proxy/http/HttpBodyFactory.cc
index 699016d..88f6b9f 100644
--- a/proxy/http/HttpBodyFactory.cc
+++ b/proxy/http/HttpBodyFactory.cc
@@ -73,6 +73,11 @@ HttpBodyFactory::fabricate_with_old_api(const char *type, HttpTransact::State *c
   bool found_requested_template = false;
   bool plain_flag               = false;
 
+  *resulting_buffer_length = 0;
+
+  ink_strlcpy(content_language_out_buf, "en", content_language_buf_size);
+  ink_strlcpy(content_type_out_buf, "text/html", content_type_buf_size);
+
   ///////////////////////////////////////////////
   // if suppressing this response, return NULL //
   ///////////////////////////////////////////////
@@ -85,11 +90,6 @@ HttpBodyFactory::fabricate_with_old_api(const char *type, HttpTransact::State *c
 
   lock();
 
-  *resulting_buffer_length = 0;
-
-  ink_strlcpy(content_language_out_buf, "en", content_language_buf_size);
-  ink_strlcpy(content_type_out_buf, "text/html", content_type_buf_size);
-
   ///////////////////////////////////////////////////////////////////
   // if logging turned on, buffer up the URL string for simplicity //
   ///////////////////////////////////////////////////////////////////
@@ -272,11 +272,6 @@ HttpBodyFactory::reconfigure()
   all_found      = all_found && (rec_err == REC_ERR_OKAY);
   Debug("body_factory", "enable_logging = %d (found = %" PRId64 ")", enable_logging, e);
 
-  rec_err                   = RecGetRecordInt("proxy.config.body_factory.response_suppression_mode", &e);
-  response_suppression_mode = ((rec_err == REC_ERR_OKAY) ? e : 0);
-  all_found                 = all_found && (rec_err == REC_ERR_OKAY);
-  Debug("body_factory", "response_suppression_mode = %d (found = %" PRId64 ")", response_suppression_mode, e);
-
   ats_scoped_str directory_of_template_sets;
 
   rec_err   = RecGetRecordString_Xmalloc("proxy.config.body_factory.template_sets_dir", &s);
@@ -345,9 +340,9 @@ HttpBodyFactory::HttpBodyFactory()
   // set up management configuration-change callbacks //
   //////////////////////////////////////////////////////
 
-  static const char *config_record_names[] = {
-    "proxy.config.body_factory.enable_customizations", "proxy.config.body_factory.enable_logging",
-    "proxy.config.body_factory.template_sets_dir", "proxy.config.body_factory.response_suppression_mode", nullptr};
+  static const char *config_record_names[] = {"proxy.config.body_factory.enable_customizations",
+                                              "proxy.config.body_factory.enable_logging",
+                                              "proxy.config.body_factory.template_sets_dir", nullptr};
 
   no_registrations_failed = true;
   for (i = 0; config_record_names[i] != nullptr; i++) {
@@ -687,11 +682,11 @@ HttpBodyFactory::is_response_suppressed(HttpTransact::State *context)
      return true;
      } else
    */
-  if (response_suppression_mode == 0) {
+  if (context->txn_conf->response_suppression_mode == 0) {
     return false;
-  } else if (response_suppression_mode == 1) {
+  } else if (context->txn_conf->response_suppression_mode == 1) {
     return true;
-  } else if (response_suppression_mode == 2) {
+  } else if (context->txn_conf->response_suppression_mode == 2) {
     return context->request_data.internal_txn;
   } else {
     return false;
diff --git a/proxy/http/HttpBodyFactory.h b/proxy/http/HttpBodyFactory.h
index 3e2f103..1803eaf 100644
--- a/proxy/http/HttpBodyFactory.h
+++ b/proxy/http/HttpBodyFactory.h
@@ -205,6 +205,8 @@ public:
                                                StrList *acpt_charset_list, float *Q_best_ptr, int *La_best_ptr, int *Lc_best_ptr,
                                                int *I_best_ptr);
 
+  bool is_response_suppressed(HttpTransact::State *context);
+
 private:
   char *fabricate(StrList *acpt_language_list, StrList *acpt_charset_list, const char *type, HttpTransact::State *context,
                   int64_t *resulting_buffer_length, const char **content_language_return, const char **content_charset_return,
@@ -213,7 +215,6 @@ private:
   const char *determine_set_by_language(StrList *acpt_language_list, StrList *acpt_charset_list);
   const char *determine_set_by_host(HttpTransact::State *context);
   HttpBodyTemplate *find_template(const char *set, const char *type, HttpBodySet **body_set_return);
-  bool is_response_suppressed(HttpTransact::State *context);
   bool
   is_sane()
   {
@@ -250,9 +251,8 @@ private:
   /////////////////////////////////////
   // manager configuration variables //
   /////////////////////////////////////
-  int enable_customizations     = 0;    // 0:no custom,1:custom,2:language-targeted
-  bool enable_logging           = true; // the user wants body factory logging
-  int response_suppression_mode = 0;    // when to suppress responses
+  int enable_customizations = 0;    // 0:no custom,1:custom,2:language-targeted
+  bool enable_logging       = true; // the user wants body factory logging
 
   ////////////////////
   // internal state //
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 9b8f372..07ed442 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1296,6 +1296,9 @@ HttpConfig::startup()
 
   HttpEstablishStaticConfigByte(c.oride.allow_half_open, "proxy.config.http.allow_half_open");
 
+  // Body factory
+  HttpEstablishStaticConfigByte(c.oride.response_suppression_mode, "proxy.config.body_factory.response_suppression_mode");
+
   // open read failure retries
   HttpEstablishStaticConfigLongLong(c.oride.max_cache_open_read_retries, "proxy.config.http.cache.max_open_read_retries");
   HttpEstablishStaticConfigLongLong(c.oride.cache_open_read_retry_time, "proxy.config.http.cache.open_read_retry_time");
@@ -1595,6 +1598,8 @@ HttpConfig::reconfigure()
 
   params->oride.allow_half_open = m_master.oride.allow_half_open;
 
+  params->oride.response_suppression_mode = m_master.oride.response_suppression_mode;
+
   // open read failure retries
   params->oride.max_cache_open_read_retries = m_master.oride.max_cache_open_read_retries;
   params->oride.cache_open_read_retry_time  = m_master.oride.cache_open_read_retry_time;
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index 457c487..a47dea6 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -601,6 +601,11 @@ struct OverridableHttpConfigParams {
   /////////////////////////////////////////////////
   MgmtByte allow_half_open = 1;
 
+  /////////////////////////////////////////////////
+  // Body factory
+  /////////////////////////////////////////////////
+  MgmtByte response_suppression_mode = 0; // proxy.config.body_factory.response_suppression_mode
+
   //////////////////////////////
   // server verification mode //
   //////////////////////////////
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index a5796c0..88a2acd 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -25,6 +25,7 @@
 #include "../ProxyTransaction.h"
 #include "HttpSM.h"
 #include "HttpTransact.h"
+#include "HttpBodyFactory.h"
 #include "HttpTransactHeaders.h"
 #include "ProxyConfig.h"
 #include "Http1ServerSession.h"
@@ -79,6 +80,8 @@
 
 extern int cache_config_read_while_writer;
 
+extern HttpBodyFactory *body_factory;
+
 // We have a debugging list that can use to find stuck
 //  state machines
 DList(HttpSM, debug_link) debug_sm_list;
@@ -6589,7 +6592,8 @@ HttpSM::setup_100_continue_transfer()
 void
 HttpSM::setup_error_transfer()
 {
-  if (t_state.internal_msg_buffer || is_response_body_precluded(t_state.http_return_code)) {
+  if (body_factory->is_response_suppressed(&t_state) || t_state.internal_msg_buffer ||
+      is_response_body_precluded(t_state.http_return_code)) {
     // Since we need to send the error message, call the API
     //   function
     ink_assert(t_state.internal_msg_buffer_size > 0 || is_response_body_precluded(t_state.http_return_code));
diff --git a/src/shared/overridable_txn_vars.cc b/src/shared/overridable_txn_vars.cc
index 84d90cf..906de8e 100644
--- a/src/shared/overridable_txn_vars.cc
+++ b/src/shared/overridable_txn_vars.cc
@@ -166,4 +166,6 @@ const std::unordered_map<std::string_view, std::tuple<const TSOverridableConfigK
      {"proxy.config.hostdb.ip_resolve", {TS_CONFIG_HTTP_HOST_RESOLUTION_PREFERENCE, TS_RECORDDATATYPE_STRING}},
      {"proxy.config.plugin.vc.default_buffer_index", {TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_INDEX, TS_RECORDDATATYPE_INT}},
      {"proxy.config.plugin.vc.default_buffer_water_mark", {TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK, TS_RECORDDATATYPE_INT}},
-     {"proxy.config.net.sock_notsent_lowat", {TS_CONFIG_NET_SOCK_NOTSENT_LOWAT, TS_RECORDDATATYPE_INT}}});
+     {"proxy.config.net.sock_notsent_lowat", {TS_CONFIG_NET_SOCK_NOTSENT_LOWAT, TS_RECORDDATATYPE_INT}},
+     {"proxy.config.body_factory.response_suppression_mode",
+      {TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE, TS_RECORDDATATYPE_INT}}});
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index 1b07e86..1e86f85 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -8947,6 +8947,10 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
   case TS_CONFIG_NET_SOCK_NOTSENT_LOWAT:
     ret = _memberp_to_generic(&overridableHttpConfig->sock_packet_notsent_lowat, conv);
     break;
+  case TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE:
+    ret = _memberp_to_generic(&overridableHttpConfig->response_suppression_mode, conv);
+    break;
+
   // This helps avoiding compiler warnings, yet detect unhandled enum members.
   case TS_CONFIG_NULL:
   case TS_CONFIG_LAST_ENTRY:
diff --git a/src/traffic_server/InkAPITest.cc b/src/traffic_server/InkAPITest.cc
index 0230012..856ea71 100644
--- a/src/traffic_server/InkAPITest.cc
+++ b/src/traffic_server/InkAPITest.cc
@@ -8699,7 +8699,8 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {
    "proxy.config.http.connect.dead.policy",
    "proxy.config.plugin.vc.default_buffer_index",
    "proxy.config.plugin.vc.default_buffer_water_mark",
-   "proxy.config.net.sock_notsent_lowat"}};
+   "proxy.config.net.sock_notsent_lowat",
+   "proxy.config.body_factory.response_suppression_mode"}};
 
 extern ClassAllocator<HttpSM> httpSMAllocator;