[trafficserver] branch 9.2.x updated: Improve option processing in cache promote (#8501)

[zw...@apache.org]

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 fea504910 Improve option processing in cache promote (#8501)
fea504910 is described below

commit fea504910b88f5b381c58e8a9c0a01f2c099c162
Author: Jeff Elsloo <el...@users.noreply.github.com>
AuthorDate: Mon Nov 15 10:15:48 2021 -0700

    Improve option processing in cache promote (#8501)
    
     * Fixes segfault scenario when unknown option(s) are encountered
    
    * Changes behavior to ignore unknown options rather than rejecting the entire config and refusing to load
    
    (cherry picked from commit 250575b3816b64c8bfd4c441a85175bdbbc0a7b4)
---
 plugins/cache_promote/configs.cc        | 14 +++++++++-----
 plugins/cache_promote/policy_manager.cc | 12 +++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/plugins/cache_promote/configs.cc b/plugins/cache_promote/configs.cc
index 30015b54e..d38d963db 100644
--- a/plugins/cache_promote/configs.cc
+++ b/plugins/cache_promote/configs.cc
@@ -42,7 +42,9 @@ static const struct option longopt[] = {
 // The destructor is responsible for returning the policy to the PolicyManager.
 PromotionConfig::~PromotionConfig()
 {
-  _manager->releasePolicy(_policy);
+  if (_policy != nullptr) {
+    _manager->releasePolicy(_policy);
+  }
 }
 
 // Parse the command line arguments to the plugin, and instantiate the appropriate policy
@@ -86,10 +88,8 @@ PromotionConfig::factory(int argc, char *argv[])
           TSDebug(PLUGIN_NAME, "internal_enabled set to true");
         } else {
           if (!_policy->parseOption(opt, optarg)) {
-            TSError("[%s] The specified policy (%s) does not support the -%c option", PLUGIN_NAME, _policy->policyName(), opt);
-            delete _policy;
-            _policy = nullptr;
-            return false;
+            TSError("[%s] The specified policy (%s) does not support the -%c option; skipping this argument", PLUGIN_NAME,
+                    _policy->policyName(), opt);
           }
         }
       } else {
@@ -99,6 +99,10 @@ PromotionConfig::factory(int argc, char *argv[])
     }
   }
 
+  if (_policy == nullptr) {
+    return false;
+  }
+
   // Coalesce any LRU policies via the LRU manager. This is a little ugly, but it makes configuration
   // easier, and order of options doesn't matter.
 
diff --git a/plugins/cache_promote/policy_manager.cc b/plugins/cache_promote/policy_manager.cc
index de3ecb0f8..2b30adb2c 100644
--- a/plugins/cache_promote/policy_manager.cc
+++ b/plugins/cache_promote/policy_manager.cc
@@ -48,7 +48,7 @@ PolicyManager::releasePolicy(PromotionPolicy *policy)
 {
   std::string tag = policy->id();
 
-  if (tag.size() != 0) {
+  if (tag.size() != 0) { // this is always the case for instances of LRUPolicy
     auto res = _policies.find(tag);
 
     if (res != _policies.end()) {
@@ -57,11 +57,13 @@ PolicyManager::releasePolicy(PromotionPolicy *policy)
         delete res->second.first;
         _policies.erase(res);
       }
+
+      return;
     } else {
-      TSAssert(!"Trying to release a policy which was not acquired via PolicyManager");
+      TSDebug(PLUGIN_NAME, "Tried to release a policy which was not properly initialized nor acquired via PolicyManager");
     }
-  } else {
-    // Not managed by the policy manager, so just nuke it.
-    delete policy;
   }
+
+  // Not managed by the policy manager, so just nuke it.
+  delete policy;
 }