[cpp-sp] branch master updated: Pull support for deprecated methods in attribute layer.

Scott Cantor cantor.2 at osu.edu
Mon Apr 23 17:07:21 EDT 2018


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

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

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

The following commit(s) were added to refs/heads/master by this push:
       new  abe60b0   Pull support for deprecated methods in attribute layer.
abe60b0 is described below

commit abe60b0893c5ccfc49af690905e2b58ba8e39574
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Apr 23 17:07:13 2018 -0400

    Pull support for deprecated methods in attribute layer.
---
 plugins/AttributeResolverHandler.cpp               | 71 +++++++++++-----------
 plugins/CaseFoldingAttributeResolver.cpp           | 14 -----
 plugins/GSSAPIAttributeExtractor.cpp               |  3 +-
 plugins/TemplateAttributeResolver.cpp              | 14 -----
 plugins/TransformAttributeResolver.cpp             | 14 -----
 shibsp/attribute/resolver/AttributeExtractor.h     | 20 +-----
 shibsp/attribute/resolver/AttributeResolver.h      | 30 +--------
 .../resolver/impl/AssertionAttributeExtractor.cpp  |  3 +-
 .../resolver/impl/ChainingAttributeExtractor.cpp   | 34 +----------
 .../resolver/impl/ChainingAttributeResolver.cpp    | 50 +--------------
 .../resolver/impl/DelegationAttributeExtractor.cpp |  4 +-
 .../impl/KeyDescriptorAttributeExtractor.cpp       |  4 +-
 .../resolver/impl/MetadataAttributeExtractor.cpp   | 10 ---
 .../resolver/impl/QueryAttributeResolver.cpp       | 41 +++++--------
 .../impl/SimpleAggregationAttributeResolver.cpp    | 36 ++++-------
 .../resolver/impl/XMLAttributeExtractor.cpp        |  7 ---
 shibsp/handler/impl/AssertionConsumerService.cpp   | 27 ++++----
 17 files changed, 89 insertions(+), 293 deletions(-)

diff --git a/plugins/AttributeResolverHandler.cpp b/plugins/AttributeResolverHandler.cpp
index 1c67548..c0af41a 100644
--- a/plugins/AttributeResolverHandler.cpp
+++ b/plugins/AttributeResolverHandler.cpp
@@ -319,13 +319,18 @@ pair<bool,long> AttributeResolverHandler::processMessage(
             v1name->setNameQualifier(nameQualifier.get());
         }
 
-        scoped_ptr<ResolutionContext> ctx;
-        ctx.reset(resolveAttributes(application, httpRequest, site.second, protocol.get(), v1name.get(), v2name.get()));
+        scoped_ptr<ResolutionContext> ctx(resolveAttributes(application, httpRequest, site.second, protocol.get(), v1name.get(), v2name.get()));
 
-        buildJSON(msg, ctx->getResolvedAttributes(), param_encoding.second);
+        if (ctx) {
+            buildJSON(msg, ctx->getResolvedAttributes(), param_encoding.second);
+        }
+        else {
+            vector<Attribute*> noattrs;
+            buildJSON(msg, noattrs, param_encoding.second);
+        }
     }
-    catch (std::exception& ex) {
-        m_log.error("error while processing request: %s", ex.what());
+    catch (const std::exception& ex) {
+        // Logging should be handled by the resolver plugin at whatever level is appropriate.        
         msg << "{}";
         return make_pair(true, httpResponse.sendError(msg));
     }
@@ -400,44 +405,40 @@ ResolutionContext* AttributeResolverHandler::resolveAttributes(
         }
     }
 
-    try {
-        AttributeResolver* resolver = application.getAttributeResolver();
-        if (resolver) {
-            m_log.debug("resolving attributes...");
-
-            Locker locker(resolver);
-            auto_ptr<ResolutionContext> ctx(
-                resolver->createResolutionContext(
-                    application,
-                    &httpRequest,
-                    issuer ? dynamic_cast<const saml2md::EntityDescriptor*>(issuer->getParent()) : nullptr,
-                    protocol,
-                    nameid,
-                    nullptr,
-                    nullptr,
-                    nullptr,
-                    &resolvedAttributes
-                    )
-                );
-            resolver->resolveAttributes(*ctx);
-            // Copy over any pushed attributes.
-            while (!resolvedAttributes.empty()) {
-                ctx->getResolvedAttributes().push_back(resolvedAttributes.back());
-                resolvedAttributes.pop_back();
-            }
-            return ctx.release();
+    AttributeResolver* resolver = application.getAttributeResolver();
+    if (resolver) {
+        m_log.debug("resolving attributes...");
+
+        Locker locker(resolver);
+        auto_ptr<ResolutionContext> ctx(
+            resolver->createResolutionContext(
+                application,
+                &httpRequest,
+                issuer ? dynamic_cast<const saml2md::EntityDescriptor*>(issuer->getParent()) : nullptr,
+                protocol,
+                nameid,
+                nullptr,
+                nullptr,
+                nullptr,
+                &resolvedAttributes
+                )
+            );
+        resolver->resolveAttributes(*ctx);
+        // Copy over any pushed attributes.
+        while (!resolvedAttributes.empty()) {
+            ctx->getResolvedAttributes().push_back(resolvedAttributes.back());
+            resolvedAttributes.pop_back();
         }
-    }
-    catch (std::exception& ex) {
-        m_log.error("attribute resolution failed: %s", ex.what());
+        return ctx.release();
     }
 
     if (!resolvedAttributes.empty()) {
         try {
             return new DummyContext(resolvedAttributes);
         }
-        catch (bad_alloc&) {
+        catch (...) {
             for_each(resolvedAttributes.begin(), resolvedAttributes.end(), xmltooling::cleanup<shibsp::Attribute>());
+            throw;
         }
     }
     return nullptr;
diff --git a/plugins/CaseFoldingAttributeResolver.cpp b/plugins/CaseFoldingAttributeResolver.cpp
index e3e03a9..a3ac716 100644
--- a/plugins/CaseFoldingAttributeResolver.cpp
+++ b/plugins/CaseFoldingAttributeResolver.cpp
@@ -89,20 +89,6 @@ namespace shibsp {
 
         ResolutionContext* createResolutionContext(
             const Application& application,
-            const opensaml::saml2md::EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const opensaml::saml2::NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<Attribute*>* attributes=nullptr
-            ) const {
-            // Make sure new method gets run.
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-        }
-
-        ResolutionContext* createResolutionContext(
-            const Application& application,
             const GenericRequest* request,
             const opensaml::saml2md::EntityDescriptor* issuer,
             const XMLCh* protocol,
diff --git a/plugins/GSSAPIAttributeExtractor.cpp b/plugins/GSSAPIAttributeExtractor.cpp
index ad86289..39e1f95 100644
--- a/plugins/GSSAPIAttributeExtractor.cpp
+++ b/plugins/GSSAPIAttributeExtractor.cpp
@@ -120,6 +120,7 @@ namespace shibsp {
 
         void extractAttributes(
             const Application& application,
+            const GenericRequest* request,
             const RoleDescriptor* issuer,
             const XMLObject& xmlObject,
             vector<Attribute*>& attributes
@@ -319,7 +320,7 @@ void GSSAPIExtractorImpl::extractAttributes(
 }
 
 void GSSAPIExtractor::extractAttributes(
-    const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
+    const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
     ) const
 {
     if (!m_impl)
diff --git a/plugins/TemplateAttributeResolver.cpp b/plugins/TemplateAttributeResolver.cpp
index cb9f828..b755641 100644
--- a/plugins/TemplateAttributeResolver.cpp
+++ b/plugins/TemplateAttributeResolver.cpp
@@ -87,20 +87,6 @@ namespace shibsp {
 
         ResolutionContext* createResolutionContext(
             const Application& application,
-            const opensaml::saml2md::EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const opensaml::saml2::NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<Attribute*>* attributes=nullptr
-            ) const {
-            // Make sure new method gets run.
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-        }
-
-        ResolutionContext* createResolutionContext(
-            const Application& application,
             const GenericRequest* request,
             const opensaml::saml2md::EntityDescriptor* issuer,
             const XMLCh* protocol,
diff --git a/plugins/TransformAttributeResolver.cpp b/plugins/TransformAttributeResolver.cpp
index 5dfcaa1..47f5087 100644
--- a/plugins/TransformAttributeResolver.cpp
+++ b/plugins/TransformAttributeResolver.cpp
@@ -89,20 +89,6 @@ namespace shibsp {
 
         ResolutionContext* createResolutionContext(
             const Application& application,
-            const opensaml::saml2md::EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const opensaml::saml2::NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<Attribute*>* attributes=nullptr
-            ) const {
-            // Make sure new method gets run.
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-        }
-
-        ResolutionContext* createResolutionContext(
-            const Application& application,
             const GenericRequest* request,
             const opensaml::saml2md::EntityDescriptor* issuer,
             const XMLCh* protocol,
diff --git a/shibsp/attribute/resolver/AttributeExtractor.h b/shibsp/attribute/resolver/AttributeExtractor.h
index 08889b8..fd0d30d 100644
--- a/shibsp/attribute/resolver/AttributeExtractor.h
+++ b/shibsp/attribute/resolver/AttributeExtractor.h
@@ -61,24 +61,6 @@ namespace shibsp {
         virtual ~AttributeExtractor();
 
         /**
-         * @deprecated
-         * Extracts the attributes found in an XMLObject.
-         *
-         * @param application   Application performing the extraction
-         * @param issuer        source of object, if known
-         * @param xmlObject     object to extract
-         * @param attributes    an array to populate with the extracted attributes
-         *
-         * @throws AttributeExtractionException thrown if there is a problem extracting attributes
-         */
-        virtual void extractAttributes(
-            const Application& application,
-            const opensaml::saml2md::RoleDescriptor* issuer,
-            const xmltooling::XMLObject& xmlObject,
-            std::vector<Attribute*>& attributes
-            ) const;
-
-        /**
          * Extracts the attributes found in an XMLObject.
          *
          * @param application   Application performing the extraction
@@ -95,7 +77,7 @@ namespace shibsp {
             const opensaml::saml2md::RoleDescriptor* issuer,
             const xmltooling::XMLObject& xmlObject,
             std::vector<Attribute*>& attributes
-            ) const;
+            ) const=0;
 
         /**
          * Populates an array with the set of Attribute IDs that might be generated.
diff --git a/shibsp/attribute/resolver/AttributeResolver.h b/shibsp/attribute/resolver/AttributeResolver.h
index fd4596f..5c34b6e 100644
--- a/shibsp/attribute/resolver/AttributeResolver.h
+++ b/shibsp/attribute/resolver/AttributeResolver.h
@@ -73,34 +73,6 @@ namespace shibsp {
         virtual ~AttributeResolver();
 
         /**
-         * @deprecated
-         * Creates a ResolutionContext based on session bootstrap material.
-         *
-         * <p>This enables resolution to occur ahead of session creation so that
-         * Attributes can be supplied while creating the session.
-         *
-         * @param application       reference to Application that owns the eventual Session
-         * @param issuer            issuing metadata of assertion issuer, if known
-         * @param protocol          protocol used to establish Session
-         * @param nameid            principal identifier, normalized to SAML 2, if any
-         * @param authncontext_class    method/category of authentication event, if known
-         * @param authncontext_decl specifics of authentication event, if known
-         * @param tokens            assertions initiating the Session, if any
-         * @param attributes        array of previously resolved attributes, if any
-         * @return  newly created ResolutionContext, owned by caller
-         */
-        virtual ResolutionContext* createResolutionContext(
-            const Application& application,
-            const opensaml::saml2md::EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const opensaml::saml2::NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const std::vector<const opensaml::Assertion*>* tokens=nullptr,
-            const std::vector<Attribute*>* attributes=nullptr
-            ) const;
-
-        /**
          * Creates a ResolutionContext based on session bootstrap material.
          *
          * <p>This enables resolution to occur ahead of session creation so that
@@ -127,7 +99,7 @@ namespace shibsp {
             const XMLCh* authncontext_decl=nullptr,
             const std::vector<const opensaml::Assertion*>* tokens=nullptr,
             const std::vector<Attribute*>* attributes=nullptr
-            ) const;
+            ) const=0;
 
         /**
          * Creates a ResolutionContext for an existing Session.
diff --git a/shibsp/attribute/resolver/impl/AssertionAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/AssertionAttributeExtractor.cpp
index c75b245..7bb5a80 100644
--- a/shibsp/attribute/resolver/impl/AssertionAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/AssertionAttributeExtractor.cpp
@@ -66,6 +66,7 @@ namespace shibsp {
 
         void extractAttributes(
             const Application& application,
+            const GenericRequest* request,
             const RoleDescriptor* issuer,
             const XMLObject& xmlObject,
             vector<shibsp::Attribute*>& attributes
@@ -113,7 +114,7 @@ AssertionExtractor::AssertionExtractor(const DOMElement* e)
 }
 
 void AssertionExtractor::extractAttributes(
-    const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<shibsp::Attribute*>& attributes
+    const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<shibsp::Attribute*>& attributes
     ) const
 {
     const saml2p::StatusResponseType* srt = dynamic_cast<const saml2p::StatusResponseType*>(&xmlObject);
diff --git a/shibsp/attribute/resolver/impl/ChainingAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/ChainingAttributeExtractor.cpp
index c03cfd8..b88e35e 100644
--- a/shibsp/attribute/resolver/impl/ChainingAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/ChainingAttributeExtractor.cpp
@@ -56,16 +56,6 @@ namespace shibsp {
 
         void extractAttributes(
             const Application& application,
-            const RoleDescriptor* issuer,
-            const XMLObject& xmlObject,
-            vector<Attribute*>& attributes
-            ) const {
-            // Make sure new version gets run.
-            extractAttributes(application, nullptr, issuer, xmlObject, attributes);
-        }
-
-        void extractAttributes(
-            const Application& application,
             const GenericRequest* request,
             const RoleDescriptor* issuer,
             const XMLObject& xmlObject,
@@ -131,28 +121,6 @@ void AttributeExtractor::generateMetadata(SPSSODescriptor& role) const
 {
 }
 
-void AttributeExtractor::extractAttributes(
-    const Application& application,
-    const GenericRequest* request,
-    const RoleDescriptor* issuer,
-    const XMLObject& xmlObject,
-    vector<Attribute*>& attributes
-    ) const
-{
-    // Default call into deprecated method.
-    extractAttributes(application, issuer, xmlObject, attributes);
-}
-
-void AttributeExtractor::extractAttributes(
-    const Application& application,
-    const RoleDescriptor* issuer,
-    const XMLObject& xmlObject,
-    vector<Attribute*>& attributes
-    ) const
-{
-    // Empty default for deprecated method.
-}
-
 ChainingAttributeExtractor::ChainingAttributeExtractor(const DOMElement* e)
 {
     SPConfig& conf = SPConfig::getConfig();
@@ -170,7 +138,7 @@ ChainingAttributeExtractor::ChainingAttributeExtractor(const DOMElement* e)
                 m_extractors.push_back(np.get());
                 np.release();
             }
-            catch (exception& ex) {
+            catch (const exception& ex) {
                 Category::getInstance(SHIBSP_LOGCAT ".AttributeExtractor.Chaining").error(
                     "caught exception processing embedded AttributeExtractor element: %s", ex.what()
                     );
diff --git a/shibsp/attribute/resolver/impl/ChainingAttributeResolver.cpp b/shibsp/attribute/resolver/impl/ChainingAttributeResolver.cpp
index 39ee374..e7e6209 100644
--- a/shibsp/attribute/resolver/impl/ChainingAttributeResolver.cpp
+++ b/shibsp/attribute/resolver/impl/ChainingAttributeResolver.cpp
@@ -113,20 +113,6 @@ namespace shibsp {
 
         ResolutionContext* createResolutionContext(
             const Application& application,
-            const EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<shibsp::Attribute*>* attributes=nullptr
-            ) const {
-            // Make sure new method gets run.
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-        }
-
-        ResolutionContext* createResolutionContext(
-            const Application& application,
             const GenericRequest* request,
             const EntityDescriptor* issuer,
             const XMLCh* protocol,
@@ -191,38 +177,6 @@ AttributeResolver::~AttributeResolver()
 {
 }
 
-ResolutionContext* AttributeResolver::createResolutionContext(
-    const Application& application,
-    const GenericRequest* request,
-    const EntityDescriptor* issuer,
-    const XMLCh* protocol,
-    const NameID* nameid,
-    const XMLCh* authncontext_class,
-    const XMLCh* authncontext_decl,
-    const vector<const opensaml::Assertion*>* tokens,
-    const vector<shibsp::Attribute*>* attributes
-    ) const
-{
-    // Default call into deprecated method.
-    return createResolutionContext(application, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-}
-
-ResolutionContext* AttributeResolver::createResolutionContext(
-    const Application& application,
-    const EntityDescriptor* issuer,
-    const XMLCh* protocol,
-    const NameID* nameid,
-    const XMLCh* authncontext_class,
-    const XMLCh* authncontext_decl,
-    const vector<const opensaml::Assertion*>* tokens,
-    const vector<shibsp::Attribute*>* attributes
-    ) const
-{
-    // Default for deprecated method.
-    throw ConfigurationException("Deprecated method implementation should always be overridden.");
-}
-
-
 ChainingAttributeResolver::ChainingAttributeResolver(const DOMElement* e)
 {
     SPConfig& conf = SPConfig::getConfig();
@@ -240,7 +194,7 @@ ChainingAttributeResolver::ChainingAttributeResolver(const DOMElement* e)
                 m_resolvers.push_back(np.get());
                 np.release();
             }
-            catch (exception& ex) {
+            catch (const exception& ex) {
                 Category::getInstance(SHIBSP_LOGCAT ".AttributeResolver." CHAINING_ATTRIBUTE_RESOLVER).error(
                     "caught exception processing embedded AttributeResolver element: %s", ex.what()
                     );
@@ -274,7 +228,7 @@ void ChainingAttributeResolver::resolveAttributes(ResolutionContext& ctx) const
             chain.m_ownedAssertions.insert(chain.m_ownedAssertions.end(), context->getResolvedAssertions().begin(), context->getResolvedAssertions().end());
             context->getResolvedAssertions().clear();
         }
-        catch (exception& ex) {
+        catch (const exception& ex) {
             Category::getInstance(SHIBSP_LOGCAT ".AttributeResolver." CHAINING_ATTRIBUTE_RESOLVER).error(
                 "caught exception applying AttributeResolver in chain: %s", ex.what()
                 );
diff --git a/shibsp/attribute/resolver/impl/DelegationAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/DelegationAttributeExtractor.cpp
index dc6c1f7..2af5a75 100644
--- a/shibsp/attribute/resolver/impl/DelegationAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/DelegationAttributeExtractor.cpp
@@ -68,7 +68,7 @@ namespace shibsp {
         }
 
         void extractAttributes(
-            const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
+            const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
             ) const;
 
         void getAttributeIds(std::vector<std::string>& attributes) const {
@@ -99,7 +99,7 @@ DelegationExtractor::DelegationExtractor(const DOMElement* e)
 }
 
 void DelegationExtractor::extractAttributes(
-    const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
+    const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
     ) const
 {
     const saml2::Assertion* assertion = dynamic_cast<const saml2::Assertion*>(&xmlObject);
diff --git a/shibsp/attribute/resolver/impl/KeyDescriptorAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/KeyDescriptorAttributeExtractor.cpp
index 545f8c7..ae6ebd0 100644
--- a/shibsp/attribute/resolver/impl/KeyDescriptorAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/KeyDescriptorAttributeExtractor.cpp
@@ -68,7 +68,7 @@ namespace shibsp {
         }
 
         void extractAttributes(
-            const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
+            const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
             ) const;
 
         void getAttributeIds(std::vector<std::string>& attributes) const {
@@ -120,7 +120,7 @@ KeyDescriptorExtractor::KeyDescriptorExtractor(const DOMElement* e) : m_hashAlg(
 }
 
 void KeyDescriptorExtractor::extractAttributes(
-    const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
+    const Application& application, const GenericRequest* request, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
     ) const
 {
     const RoleDescriptor* role = dynamic_cast<const RoleDescriptor*>(&xmlObject);
diff --git a/shibsp/attribute/resolver/impl/MetadataAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/MetadataAttributeExtractor.cpp
index 208c853..fbaba22 100644
--- a/shibsp/attribute/resolver/impl/MetadataAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/MetadataAttributeExtractor.cpp
@@ -67,16 +67,6 @@ namespace shibsp {
         void unlock() {
         }
 
-        // deprecated
-        void extractAttributes(
-            const Application& application,
-            const RoleDescriptor* issuer,
-            const XMLObject& xmlObject,
-            vector<shibsp::Attribute*>& attributes
-            ) const {
-            extractAttributes(application, nullptr, issuer, xmlObject, attributes);
-        }
-
         void extractAttributes(
             const Application& application,
             const GenericRequest* request,
diff --git a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
index 1980f45..bcbd213 100644
--- a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
+++ b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
@@ -188,20 +188,6 @@ namespace shibsp {
         Lockable* lock() {return this;}
         void unlock() {}
 
-        // deprecated method
-        ResolutionContext* createResolutionContext(
-            const Application& application,
-            const EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<shibsp::Attribute*>* attributes=nullptr
-            ) const {
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens);
-        }
-
         ResolutionContext* createResolutionContext(
             const Application& application,
             const GenericRequest* request,
@@ -277,7 +263,7 @@ QueryResolver::QueryResolver(const DOMElement* e)
                 }
             }
         }
-        catch (exception& ex) {
+        catch (const exception& ex) {
             m_log.error("exception loading attribute designator: %s", ex.what());
         }
         child = XMLHelper::getNextSiblingElement(child);
@@ -299,7 +285,7 @@ void QueryResolver::SAML1Query(QueryContext& ctx) const
         find_if(ctx.getEntityDescriptor()->getAttributeAuthorityDescriptors(), isValidForProtocol(ctx.getProtocol()));
     if (!AA) {
         m_log.warn("no SAML 1.%d AttributeAuthority role found in metadata", version);
-        return;
+        throw MetadataException("Unable to locate SAML 1 AttributeAuthority role.");
     }
 
     const Application& application = ctx.getApplication();
@@ -347,7 +333,7 @@ void QueryResolver::SAML1Query(QueryContext& ctx) const
             client.sendSAML(request, application.getId(), mcc, loc.get());
             response.reset(client.receiveSAML());
         }
-        catch (exception& ex) {
+        catch (const exception& ex) {
             m_log.error("exception during SAML query to %s: %s", loc.get(), ex.what());
             soaper.reset();
         }
@@ -393,7 +379,7 @@ void QueryResolver::SAML1Query(QueryContext& ctx) const
         if (!policy->isAuthenticated())
             throw SecurityPolicyException("Security of SAML 1.x query result not established.");
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         m_log.error("assertion failed policy validation: %s", ex.what());
         throw;
     }
@@ -435,7 +421,7 @@ void QueryResolver::SAML1Query(QueryContext& ctx) const
             filter->filterAttributes(fc, ctx.getResolvedAttributes());
         }
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         m_log.error("caught exception extracting/filtering attributes from query result: %s", ex.what());
         for_each(ctx.getResolvedAttributes().begin(), ctx.getResolvedAttributes().end(), xmltooling::cleanup<shibsp::Attribute>());
         ctx.getResolvedAttributes().clear();
@@ -453,7 +439,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
         find_if(ctx.getEntityDescriptor()->getAttributeAuthorityDescriptors(), isValidForProtocol(samlconstants::SAML20P_NS));
     if (!AA) {
         m_log.warn("no SAML 2 AttributeAuthority role found in metadata");
-        return;
+        throw MetadataException("Unable to locate SAML 2.0 AttributeAuthority role.");
     }
 
     const Application& application = ctx.getApplication();
@@ -497,7 +483,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
                     subject->setEncryptedID(encrypted.get());
                     encrypted.release();
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     // If we're encrypting deliberately, failure should be fatal.
                     if (encryption.first && strcmp(encryption.second, "conditional")) {
                         throw;
@@ -530,7 +516,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
             client.sendSAML(query, application.getId(), mcc, loc.get());
             srt.reset(client.receiveSAML());
         }
-        catch (exception& ex) {
+        catch (const exception& ex) {
             m_log.error("exception during SAML query to %s: %s", loc.get(), ex.what());
             soaper.reset();
         }
@@ -594,7 +580,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
                     m_log.debugStream() << "decrypted assertion: " << *newtoken << logging::eol;
             }
         }
-        catch (exception& ex) {
+        catch (const exception& ex) {
             m_log.error("failed to decrypt assertion: %s", ex.what());
             throw;
         }
@@ -661,7 +647,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
             }
         }
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         m_log.error("assertion failed policy validation: %s", ex.what());
         throw;
     }
@@ -690,7 +676,7 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const
             filter->filterAttributes(fc, ctx.getResolvedAttributes());
         }
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         m_log.error("caught exception extracting/filtering attributes from query result: %s", ex.what());
         for_each(ctx.getResolvedAttributes().begin(), ctx.getResolvedAttributes().end(), xmltooling::cleanup<shibsp::Attribute>());
         ctx.getResolvedAttributes().clear();
@@ -729,7 +715,7 @@ void QueryResolver::resolveAttributes(ResolutionContext& ctx) const
             m_log.warn("can't attempt attribute query, either no NameID or no metadata to use");
         }
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         // Already logged.
         if (!m_exceptionId.empty()) {
             auto_ptr<SimpleAttribute> attr(new SimpleAttribute(m_exceptionId));
@@ -737,5 +723,8 @@ void QueryResolver::resolveAttributes(ResolutionContext& ctx) const
             qctx.getResolvedAttributes().push_back(attr.get());
             attr.release();
         }
+        else {
+            throw; // not exposing the exception as an attribute, so just surface to caller
+        }
     }
 }
diff --git a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
index e4068d7..00b42c4 100644
--- a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
+++ b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
@@ -166,20 +166,6 @@ namespace shibsp {
         Lockable* lock() {return this;}
         void unlock() {}
 
-        // deprecated method
-        ResolutionContext* createResolutionContext(
-            const Application& application,
-            const EntityDescriptor* issuer,
-            const XMLCh* protocol,
-            const NameID* nameid=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const vector<const opensaml::Assertion*>* tokens=nullptr,
-            const vector<shibsp::Attribute*>* attributes=nullptr
-            ) const {
-            return createResolutionContext(application, nullptr, issuer, protocol, nameid, authncontext_class, authncontext_decl, tokens, attributes);
-        }
-
         ResolutionContext* createResolutionContext(
             const Application& application,
             const GenericRequest* request,
@@ -330,7 +316,7 @@ SimpleAggregationResolver::SimpleAggregationResolver(const DOMElement* e)
                     obj.release();
                 }
             }
-            catch (std::exception& ex) {
+            catch (const std::exception& ex) {
                 m_log.error("exception loading attribute designator: %s", ex.what());
             }
         }
@@ -351,11 +337,11 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
         (m_metadata ? m_metadata.get() : application.getMetadataProvider())->getEntityDescriptor(mc);
     if (!mdresult.first) {
         m_log.warn("unable to locate metadata for provider (%s)", entityID);
-        return;
+        throw MetadataException("Unable to locate metadata for provider ($entityID)", namedparams(1, "entityID", entityID));
     }
     else if (!(AA=dynamic_cast<const AttributeAuthorityDescriptor*>(mdresult.second))) {
         m_log.warn("no SAML 2 AttributeAuthority role found in metadata for (%s)", entityID);
-        return;
+        throw MetadataException("Unable to locate SAML 2.0 AttributeAuthority role for provider ($entityID)", namedparams(1, "entityID", entityID));
     }
 
     const PropertySet* relyingParty = application.getRelyingParty(mdresult.first);
@@ -403,7 +389,7 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
                     subject->setEncryptedID(encrypted.get());
                     encrypted.release();
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     // If we're encrypting deliberately, failure should be fatal.
                     if (encryption.first && strcmp(encryption.second, "conditional")) {
                         throw;
@@ -436,7 +422,7 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
             client.sendSAML(query, application.getId(), mcc, loc.get());
             srt.reset(client.receiveSAML());
         }
-        catch (std::exception& ex) {
+        catch (const std::exception& ex) {
             m_log.error("exception during SAML query to %s: %s", loc.get(), ex.what());
             soaper.reset();
         }
@@ -501,7 +487,7 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
                     m_log.debugStream() << "decrypted assertion: " << *newtoken << logging::eol;
             }
         }
-        catch (std::exception& ex) {
+        catch (const std::exception& ex) {
             m_log.error("failed to decrypt assertion: %s", ex.what());
             throw;
         }
@@ -568,7 +554,7 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
             }
         }
     }
-    catch (std::exception& ex) {
+    catch (const std::exception& ex) {
         m_log.error("assertion failed policy validation: %s", ex.what());
         throw;
     }
@@ -597,7 +583,7 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
             filter->filterAttributes(fc, ctx.getResolvedAttributes());
         }
     }
-    catch (std::exception& ex) {
+    catch (const std::exception& ex) {
         m_log.error("caught exception extracting/filtering attributes from query result: %s", ex.what());
         for_each(ctx.getResolvedAttributes().begin(), ctx.getResolvedAttributes().end(), xmltooling::cleanup<shibsp::Attribute>());
         ctx.getResolvedAttributes().clear();
@@ -703,7 +689,7 @@ void SimpleAggregationResolver::resolveAttributes(ResolutionContext& ctx) const
                 try {
                     doQuery(qctx, source->first.c_str(), n ? n.get() : qctx.getNameID());
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     if (exceptAttr.get())
                         exceptAttr->getValues().push_back(XMLToolingConfig::getConfig().getURLEncoder()->encode(ex.what()));
                 }
@@ -727,7 +713,7 @@ void SimpleAggregationResolver::resolveAttributes(ResolutionContext& ctx) const
                             try {
                                 doQuery(qctx, link->c_str(), n ? n.get() : qctx.getNameID());
                             }
-                            catch (std::exception& ex) {
+                            catch (const std::exception& ex) {
                                 if (exceptAttr.get())
                                     exceptAttr->getValues().push_back(XMLToolingConfig::getConfig().getURLEncoder()->encode(ex.what()));
                             }
@@ -752,7 +738,7 @@ void SimpleAggregationResolver::resolveAttributes(ResolutionContext& ctx) const
                                 try {
                                     doQuery(qctx, link->c_str(), n ? n.get() : qctx.getNameID());
                                 }
-                                catch (std::exception& ex) {
+                                catch (const std::exception& ex) {
                                     if (exceptAttr.get())
                                         exceptAttr->getValues().push_back(XMLToolingConfig::getConfig().getURLEncoder()->encode(ex.what()));
                                 }
diff --git a/shibsp/attribute/resolver/impl/XMLAttributeExtractor.cpp b/shibsp/attribute/resolver/impl/XMLAttributeExtractor.cpp
index 83de1d1..f52b928 100644
--- a/shibsp/attribute/resolver/impl/XMLAttributeExtractor.cpp
+++ b/shibsp/attribute/resolver/impl/XMLAttributeExtractor.cpp
@@ -164,13 +164,6 @@ namespace shibsp {
             shutdown();
         }
 
-        // deprecated method
-        void extractAttributes(
-            const Application& application, const RoleDescriptor* issuer, const XMLObject& xmlObject, vector<Attribute*>& attributes
-            ) const {
-            extractAttributes(application, nullptr, issuer, xmlObject, attributes);
-        }
-
         void extractAttributes(const Application&, const GenericRequest*, const RoleDescriptor*, const XMLObject&, vector<Attribute*>&) const;
 
         void getAttributeIds(std::vector<std::string>& attributes) const {
diff --git a/shibsp/handler/impl/AssertionConsumerService.cpp b/shibsp/handler/impl/AssertionConsumerService.cpp
index 476e378..001e806 100644
--- a/shibsp/handler/impl/AssertionConsumerService.cpp
+++ b/shibsp/handler/impl/AssertionConsumerService.cpp
@@ -214,7 +214,7 @@ pair<bool,long> AssertionConsumerService::processMessage(
                     recoverRelayState(application, httpRequest, httpResponse, relayState, false);
                     hook += "&target=" + encoder->encode(relayState.c_str());
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     m_log.warn("error recovering relay state: %s", ex.what());
                 }
             }
@@ -230,7 +230,7 @@ pair<bool,long> AssertionConsumerService::processMessage(
             try {
                 recoverRelayState(application, httpRequest, httpResponse, relayState, false);
             }
-            catch (std::exception& rsex) {
+            catch (const std::exception& rsex) {
                 m_log.warn("error recovering relay state: %s", rsex.what());
                 relayState.erase();
                 recoverRelayState(application, httpRequest, httpResponse, relayState, false);
@@ -275,7 +275,7 @@ pair<bool,long> AssertionConsumerService::processMessage(
                 m_log.warn("unable to audit event, log event object was of an incorrect type");
             }
         }
-        catch (std::exception& ex2) {
+        catch (const std::exception& ex2) {
             m_log.warn("exception auditing event: %s", ex2.what());
         }
 
@@ -449,7 +449,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
                             *id = mprefix.second + *id;
                     }
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     m_log.error("caught exception extracting attributes: %s", ex.what());
                 }
             }
@@ -461,7 +461,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
             try {
                 extractor->extractAttributes(application, request, issuer, *protmsg, resolvedAttributes);
             }
-            catch (std::exception& ex) {
+            catch (const std::exception& ex) {
                 m_log.error("caught exception extracting attributes: %s", ex.what());
             }
         }
@@ -473,7 +473,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
                 else
                     extractor->extractAttributes(application, request, issuer, *nameid, resolvedAttributes);
             }
-            catch (std::exception& ex) {
+            catch (const std::exception& ex) {
                 m_log.error("caught exception extracting attributes: %s", ex.what());
             }
         }
@@ -485,7 +485,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
                 else
                     extractor->extractAttributes(application, request, issuer, *statement, resolvedAttributes);
             }
-            catch (std::exception& ex) {
+            catch (const std::exception& ex) {
                 m_log.error("caught exception extracting attributes: %s", ex.what());
             }
         }
@@ -496,7 +496,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
                 try {
                     extractor->extractAttributes(application, request, issuer, *t, resolvedAttributes);
                 }
-                catch (std::exception& ex) {
+                catch (const std::exception& ex) {
                     m_log.error("caught exception extracting attributes: %s", ex.what());
                 }
             }
@@ -509,7 +509,7 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
             try {
                 filter->filterAttributes(fc, resolvedAttributes);
             }
-            catch (std::exception& ex) {
+            catch (const std::exception& ex) {
                 m_log.error("caught exception filtering attributes: %s", ex.what());
                 m_log.error("dumping extracted attributes due to filtering exception");
                 for_each(resolvedAttributes.begin(), resolvedAttributes.end(), xmltooling::cleanup<shibsp::Attribute>());
@@ -549,16 +549,17 @@ ResolutionContext* AssertionConsumerService::resolveAttributes(
             return ctx.release();
         }
     }
-    catch (std::exception& ex) {
-        m_log.error("attribute resolution failed: %s", ex.what());
+    catch (const std::exception&) {
+        // Logging should be handled by the resolver plugin at whatever level is appropriate.
     }
 
     if (!resolvedAttributes.empty()) {
         try {
             return new DummyContext(resolvedAttributes);
         }
-        catch (bad_alloc&) {
+        catch (...) {
             for_each(resolvedAttributes.begin(), resolvedAttributes.end(), xmltooling::cleanup<shibsp::Attribute>());
+            throw;
         }
     }
     return nullptr;
@@ -627,7 +628,7 @@ LoginEvent* AssertionConsumerService::newLoginEvent(const Application& applicati
             m_log.warn("unable to audit event, log event object was of an incorrect type");
         }
     }
-    catch (std::exception& ex) {
+    catch (const std::exception& ex) {
         m_log.warn("exception auditing event: %s", ex.what());
     }
     return nullptr;

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


More information about the commits mailing list