[cpp-sp] 01/02: Move some hardcoded names into constants.

Scott Cantor cantor.2 at osu.edu
Tue Jan 14 18:09:57 UTC 2025


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=9d3b5ee3b21d6b3c9aa44ded8b1d42aa5ccba30c

commit 9d3b5ee3b21d6b3c9aa44ded8b1d42aa5ccba30c
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jan 14 12:40:05 2025 -0500

    Move some hardcoded names into constants.
---
 shibsp/platform/iis/ModuleConfig.cpp     | 35 ++++++++++++++++++++++++--------
 shibsp/platform/iis/ModuleConfig.h       | 15 ++++++++++++++
 shibsp/util/BoostPropertySet.cpp         |  4 +++-
 shibsp/util/BoostPropertySet.h           |  3 +++
 tests/platform/iis/ModuleConfigTests.cpp | 34 +++++++++++++++----------------
 tests/util/BoostPropertySetTests.cpp     | 18 ++++++++--------
 tests/util/PropertyTreeTests.cpp         |  2 +-
 7 files changed, 75 insertions(+), 36 deletions(-)

diff --git a/shibsp/platform/iis/ModuleConfig.cpp b/shibsp/platform/iis/ModuleConfig.cpp
index bce7903b..ed54cefb 100644
--- a/shibsp/platform/iis/ModuleConfig.cpp
+++ b/shibsp/platform/iis/ModuleConfig.cpp
@@ -54,6 +54,19 @@ namespace {
 
 };
 
+const char ModuleConfig::USE_VARIABLES_PROP_NAME[] = "useVariables";
+const char ModuleConfig::USE_HEADERS_PROP_NAME[] = "useHeaders";
+const char ModuleConfig::AUTHENTICATED_ROLE_PROP_NAME[] = "authenticatedRole";
+const char ModuleConfig::ROLE_ATTRIBUTES_PROP_NAME[] = "roleAttributes";
+const char ModuleConfig::NORMALIZE_REQUEST_PROP_NAME[] = "normalizeRequest";
+const char ModuleConfig::SAFE_HEADER_NAMES_PROP_NAME[] = "safeHeaderNames";
+
+const char ModuleConfig::SITE_NAME_PROP_NAME[] = "name";
+const char ModuleConfig::SITE_SCHEME_PROP_NAME[] = "scheme";
+const char ModuleConfig::SITE_PORT_PROP_NAME[] = "port";
+const char ModuleConfig::SITE_SSLPORT_PROP_NAME[] = "sslport";
+const char ModuleConfig::SITE_ALIASES_PROP_NAME[] = "aliases";
+
 ModuleConfig::ModuleConfig() {}
 
 ModuleConfig::~ModuleConfig() {}
@@ -68,15 +81,19 @@ ModuleConfigImpl::ModuleConfigImpl(unique_ptr<ptree> pt, bool xml)
         // Migrate Roles element's attributes to this child for compatibility with INI format.
         const boost::optional<ptree&> roles = child.get_child_optional("Roles");
         if (roles) {
-            const boost::optional<ptree&> xmlattr = roles->get_child_optional("<xmlattr>");
+            const boost::optional<ptree&> xmlattr = roles->get_child_optional(XMLATTR_NODE_NAME);
             if (xmlattr) {
                 boost::optional<string> prop = xmlattr->get_optional<string>("authNRole");
                 if (prop) {
-                    child.add("<xmlattr>.authenticatedRole", *prop);
+                    string propname(XMLATTR_NODE_NAME);
+                    propname = propname + '.' + AUTHENTICATED_ROLE_PROP_NAME;
+                    child.add(propname, *prop);
                 }
-                prop = xmlattr->get_optional<string>("roleAttributes");
+                prop = xmlattr->get_optional<string>(ROLE_ATTRIBUTES_PROP_NAME);
                 if (prop) {
-                    child.add("<xmlattr>.roleAttributes", *prop);
+                    string propname(XMLATTR_NODE_NAME);
+                    propname = propname + '.' + ROLE_ATTRIBUTES_PROP_NAME;
+                    child.add(propname, *prop);
                 }
             }
         }
@@ -124,20 +141,22 @@ void ModuleConfigImpl::doSites(ptree& parent)
                 }
             }
             if (!aliases.empty()) {
-                child.second.add("<xmlattr>.aliases", aliases);
+                string propname(XMLATTR_NODE_NAME);
+                propname = propname + '.' + SITE_ALIASES_PROP_NAME;
+                child.second.add(propname, aliases);
             }
 
             m_sites[id] = std::move(propset);
             m_log.info("installed Site mapping for (%s)", id);
         }
-        else if (child.first == "<xmlattr>" || child.first == "Roles") {
+        else if (child.first == XMLATTR_NODE_NAME || child.first == "Roles") {
             continue;
         }
         else {
             // This is assumed to be an INI format site section. If not, so be it.
 
-            if (!child.second.get_child_optional("name").has_value()) {
-                m_log.warn("ignoring Site section (%s) with no 'name' property", child.first.c_str());
+            if (!child.second.get_child_optional(SITE_NAME_PROP_NAME).has_value()) {
+                m_log.warn("ignoring Site section (%s) with no '%s' property", child.first.c_str(), SITE_NAME_PROP_NAME);
                 continue;
             }
 
diff --git a/shibsp/platform/iis/ModuleConfig.h b/shibsp/platform/iis/ModuleConfig.h
index b290f169..19311440 100644
--- a/shibsp/platform/iis/ModuleConfig.h
+++ b/shibsp/platform/iis/ModuleConfig.h
@@ -46,6 +46,21 @@ namespace shibsp {
              * @param path optional path to config file to load
              */
             static std::unique_ptr<ModuleConfig> newModuleConfig(const char* path=nullptr);
+
+            // Global
+            static const char USE_VARIABLES_PROP_NAME[];
+            static const char USE_HEADERS_PROP_NAME[];
+            static const char AUTHENTICATED_ROLE_PROP_NAME[];
+            static const char ROLE_ATTRIBUTES_PROP_NAME[];
+            static const char NORMALIZE_REQUEST_PROP_NAME[];
+            static const char SAFE_HEADER_NAMES_PROP_NAME[];
+
+            // Site
+            static const char SITE_NAME_PROP_NAME[];
+            static const char SITE_SCHEME_PROP_NAME[];
+            static const char SITE_PORT_PROP_NAME[];
+            static const char SITE_SSLPORT_PROP_NAME[];
+            static const char SITE_ALIASES_PROP_NAME[];
         };
     };
 };
diff --git a/shibsp/util/BoostPropertySet.cpp b/shibsp/util/BoostPropertySet.cpp
index 576c0471..75a35397 100644
--- a/shibsp/util/BoostPropertySet.cpp
+++ b/shibsp/util/BoostPropertySet.cpp
@@ -38,6 +38,8 @@ PropertySet::~PropertySet()
 {
 }
 
+const char BoostPropertySet::XMLATTR_NODE_NAME[] = "<xmlattr>";
+
 BoostPropertySet::BoostPropertySet() : m_parent(nullptr), m_pt(nullptr)
 {
 }
@@ -59,7 +61,7 @@ void BoostPropertySet::setParent(const PropertySet* parent)
 void BoostPropertySet::load(const ptree& pt, const char* unsetter)
 {
     // Check for <xmlattr> in case this was an XML-based tree.
-    const boost::optional<const ptree&> xmlattr = pt.get_child_optional("<xmlattr>");
+    const boost::optional<const ptree&> xmlattr = pt.get_child_optional(XMLATTR_NODE_NAME);
     if (xmlattr) {
         m_pt = &xmlattr.get();
     }
diff --git a/shibsp/util/BoostPropertySet.h b/shibsp/util/BoostPropertySet.h
index cecc0041..e8ff851b 100644
--- a/shibsp/util/BoostPropertySet.h
+++ b/shibsp/util/BoostPropertySet.h
@@ -63,6 +63,9 @@ namespace shibsp {
          */
         void load(const boost::property_tree::ptree& pt, const char* unsetter=nullptr);
 
+        /** XML-based property trees contain a sub-tree of attributes under this child node. */
+        static const char XMLATTR_NODE_NAME[];
+
     protected:
         /**
          * Returns the parent PropertySet.
diff --git a/tests/platform/iis/ModuleConfigTests.cpp b/tests/platform/iis/ModuleConfigTests.cpp
index 0f609997..f0beed26 100644
--- a/tests/platform/iis/ModuleConfigTests.cpp
+++ b/tests/platform/iis/ModuleConfigTests.cpp
@@ -81,31 +81,31 @@ void validateSites(const ModuleConfig* config)
 
     const PropertySet* one = config->getSiteConfig("1");
     BOOST_CHECK(one);
-    BOOST_CHECK_EQUAL(one->getString("name"), "sp.example.org");
-    BOOST_CHECK_EQUAL(one->getString("scheme"), nullptr);
-    BOOST_CHECK_EQUAL(one->getUnsignedInt("port", 0), 0);
-    BOOST_CHECK_EQUAL(one->getString("aliases"), nullptr);
+    BOOST_CHECK_EQUAL(one->getString(ModuleConfig::SITE_NAME_PROP_NAME), "sp.example.org");
+    BOOST_CHECK_EQUAL(one->getString(ModuleConfig::SITE_SCHEME_PROP_NAME), nullptr);
+    BOOST_CHECK_EQUAL(one->getUnsignedInt(ModuleConfig::SITE_PORT_PROP_NAME, 0), 0);
+    BOOST_CHECK_EQUAL(one->getString(ModuleConfig::SITE_ALIASES_PROP_NAME), nullptr);
 
     const PropertySet* two = config->getSiteConfig("2");
     BOOST_CHECK(two);
-    BOOST_CHECK_EQUAL(two->getString("name"), "sp2.example.org");
-    BOOST_CHECK_EQUAL(two->getString("scheme"), "https");
-    BOOST_CHECK_EQUAL(two->getUnsignedInt("port", 0), 443);
-    BOOST_CHECK_EQUAL(two->getString("aliases"), nullptr);
+    BOOST_CHECK_EQUAL(two->getString(ModuleConfig::SITE_NAME_PROP_NAME), "sp2.example.org");
+    BOOST_CHECK_EQUAL(two->getString(ModuleConfig::SITE_SCHEME_PROP_NAME), "https");
+    BOOST_CHECK_EQUAL(two->getUnsignedInt(ModuleConfig::SITE_PORT_PROP_NAME, 0), 443);
+    BOOST_CHECK_EQUAL(two->getString(ModuleConfig::SITE_ALIASES_PROP_NAME), nullptr);
 
     const PropertySet* three = config->getSiteConfig("3");
     BOOST_CHECK(three);
-    BOOST_CHECK_EQUAL(three->getString("name"), "sp3.example.org");
-    BOOST_CHECK_EQUAL(three->getString("scheme"), nullptr);
-    BOOST_CHECK_EQUAL(three->getUnsignedInt("port", 0), 0);
-    BOOST_CHECK_EQUAL(three->getString("aliases"), "alt.example.org alt2.example.org");
+    BOOST_CHECK_EQUAL(three->getString(ModuleConfig::SITE_NAME_PROP_NAME), "sp3.example.org");
+    BOOST_CHECK_EQUAL(three->getString(ModuleConfig::SITE_SCHEME_PROP_NAME), nullptr);
+    BOOST_CHECK_EQUAL(three->getUnsignedInt(ModuleConfig::SITE_PORT_PROP_NAME, 0), 0);
+    BOOST_CHECK_EQUAL(three->getString(ModuleConfig::SITE_ALIASES_PROP_NAME), "alt.example.org alt2.example.org");
 }
 
 BOOST_FIXTURE_TEST_CASE(ModuleConfigTest_ini, ModuleConfigFixture)
 {
     unique_ptr<ModuleConfig> config(ModuleConfig::newModuleConfig(string(data_path + "iis.ini").c_str()));
     
-    BOOST_CHECK(config->getBool("useVariables", true));
+    BOOST_CHECK(config->getBool(ModuleConfig::USE_VARIABLES_PROP_NAME, true));
     
     validateSites(config.get());
 }
@@ -114,10 +114,10 @@ BOOST_FIXTURE_TEST_CASE(ModuleConfigTest_xml, ModuleConfigFixture)
 {
     unique_ptr<ModuleConfig> config(ModuleConfig::newModuleConfig(string(data_path + "iis.xml").c_str()));
     
-    BOOST_CHECK(!config->getBool("useVariables", true));
-    BOOST_CHECK(config->getBool("useHeaders", false));
-    BOOST_CHECK_EQUAL(config->getString("authenticatedRole"), nullptr);
-    BOOST_CHECK_EQUAL(config->getString("roleAttributes"), "foo bar");
+    BOOST_CHECK(!config->getBool(ModuleConfig::USE_VARIABLES_PROP_NAME, true));
+    BOOST_CHECK(config->getBool(ModuleConfig::USE_HEADERS_PROP_NAME, false));
+    BOOST_CHECK_EQUAL(config->getString(ModuleConfig::AUTHENTICATED_ROLE_PROP_NAME), nullptr);
+    BOOST_CHECK_EQUAL(config->getString(ModuleConfig::ROLE_ATTRIBUTES_PROP_NAME), "foo bar");
 
     validateSites(config.get());
 }
diff --git a/tests/util/BoostPropertySetTests.cpp b/tests/util/BoostPropertySetTests.cpp
index 1abd07bd..f389fd9a 100644
--- a/tests/util/BoostPropertySetTests.cpp
+++ b/tests/util/BoostPropertySetTests.cpp
@@ -13,7 +13,7 @@
  */
 
 /**
- * BoostPropertySetTests.cpp
+ * util/BoostPropertySetTests.cpp
  *
  * Unit tests for BoostPropertySet usage.
  */
@@ -82,7 +82,7 @@ BOOST_FIXTURE_TEST_CASE(BoostPropertySet_tree, BPS_Fixture)
 
     const pt::ptree& root = tree.get_child("root");
     TestBoostPropertySet rootset;
-    const boost::optional<const pt::ptree&> xmlattr = root.get_child_optional("<xmlattr>");
+    const boost::optional<const pt::ptree&> xmlattr = root.get_child_optional(BoostPropertySet::XMLATTR_NODE_NAME);
     if (xmlattr) {
         rootset.load(xmlattr.get(), "unset");
     }
@@ -91,7 +91,7 @@ BOOST_FIXTURE_TEST_CASE(BoostPropertySet_tree, BPS_Fixture)
         if (child.first == "one") {
             ones.push_back(unique_ptr<TestBoostPropertySet>(new TestBoostPropertySet()));
             const auto& one = ones.back();
-            const boost::optional<const pt::ptree&> xmlattr = child.second.get_child_optional("<xmlattr>");
+            const boost::optional<const pt::ptree&> xmlattr = child.second.get_child_optional(BoostPropertySet::XMLATTR_NODE_NAME);
             if (xmlattr) {
                 one->load(xmlattr.get(), "unset");
             }
@@ -101,7 +101,7 @@ BOOST_FIXTURE_TEST_CASE(BoostPropertySet_tree, BPS_Fixture)
                 if (child2.first == "two") {
                     twos.push_back(unique_ptr<TestBoostPropertySet>(new TestBoostPropertySet()));
                     const auto& two = twos.back();
-                    const boost::optional<const pt::ptree&> xmlattr = child2.second.get_child_optional("<xmlattr>");
+                    const boost::optional<const pt::ptree&> xmlattr = child2.second.get_child_optional(BoostPropertySet::XMLATTR_NODE_NAME);
                     if (xmlattr) {
                         two->load(xmlattr.get(), "unset");
                     }
@@ -115,25 +115,25 @@ BOOST_FIXTURE_TEST_CASE(BoostPropertySet_tree, BPS_Fixture)
     BOOST_CHECK_EQUAL(twos.size(), 2);
     BOOST_CHECK_EQUAL(rootset.getString("foo"), "bar");
     BOOST_CHECK_EQUAL(rootset.getString("zork"), "frobnitz");
-    BOOST_CHECK_EQUAL(rootset.getString("<xmlattr>"), nullptr);
+    BOOST_CHECK_EQUAL(rootset.getString(BoostPropertySet::XMLATTR_NODE_NAME), nullptr);
     BOOST_CHECK_EQUAL(rootset.getString("one"), nullptr);
 
     BOOST_CHECK_EQUAL(ones[0]->getString("foo"), "baz");
     BOOST_CHECK_EQUAL(ones[0]->getString("zork"), "frobnitz");
-    BOOST_CHECK_EQUAL(ones[0]->getString("<xmlattr>"), nullptr);
+    BOOST_CHECK_EQUAL(ones[0]->getString(BoostPropertySet::XMLATTR_NODE_NAME), nullptr);
     BOOST_CHECK_EQUAL(ones[0]->getString("two"), nullptr);
 
     BOOST_CHECK_EQUAL(ones[1]->getString("foo", "zork"), "zork");
     BOOST_CHECK_EQUAL(ones[1]->getString("zork"), nullptr);
-    BOOST_CHECK_EQUAL(ones[1]->getString("<xmlattr>"), nullptr);
+    BOOST_CHECK_EQUAL(ones[1]->getString(BoostPropertySet::XMLATTR_NODE_NAME), nullptr);
     BOOST_CHECK_EQUAL(ones[1]->getString("two"), nullptr);
 
     BOOST_CHECK_EQUAL(twos[0]->getString("foo"), "baz");
     BOOST_CHECK_EQUAL(twos[0]->getString("zork"), "frobnitz");
-    BOOST_CHECK_EQUAL(twos[0]->getString("<xmlattr>"), nullptr);
+    BOOST_CHECK_EQUAL(twos[0]->getString(BoostPropertySet::XMLATTR_NODE_NAME), nullptr);
 
     BOOST_CHECK_EQUAL(twos[1]->getString("unset"), "foo zork");
     BOOST_CHECK_EQUAL(twos[1]->getString("foo"), nullptr);
     BOOST_CHECK_EQUAL(twos[1]->getString("zork"), "zorkmid");
-    BOOST_CHECK_EQUAL(twos[1]->getString("<xmlattr>"), nullptr);
+    BOOST_CHECK_EQUAL(twos[1]->getString(BoostPropertySet::XMLATTR_NODE_NAME), nullptr);
 }
diff --git a/tests/util/PropertyTreeTests.cpp b/tests/util/PropertyTreeTests.cpp
index 9a5951e3..1da87a21 100644
--- a/tests/util/PropertyTreeTests.cpp
+++ b/tests/util/PropertyTreeTests.cpp
@@ -13,7 +13,7 @@
  */
 
 /**
- * PropertyTreeTests.cpp
+ * util/PropertyTreeTests.cpp
  *
  * Unit tests for property tree usage.
  */

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list