[project @ Message->getArg & friends return FailureResponse on error]

5 files changed, 72 insertions, 46 deletions

diff --git a/Auth/OpenID/AX.php b/Auth/OpenID/AX.php
index db7a5ca..4a617ae 100644
--- a/Auth/OpenID/AX.php
+++ b/Auth/OpenID/AX.php
@@ -930,7 +930,8 @@ class Auth_OpenID_AX_FetchResponse extends Auth_OpenID_AX_KeyValueMessage {
         } else {
             $ax_args = $success_response->message->getArgs($obj->ns_uri);
         }
-        if ($ax_args === null || sizeof($ax_args) == 0) {
+        if ($ax_args === null || Auth_OpenID::isFailure($ax_args) ||
+              sizeof($ax_args) == 0) {
             return null;
         }
 
diff --git a/Auth/OpenID/Association.php b/Auth/OpenID/Association.php
index 3bae6a9..602c20a 100644
--- a/Auth/OpenID/Association.php
+++ b/Auth/OpenID/Association.php
@@ -330,7 +330,7 @@ class Auth_OpenID_Association {
     function _makePairs(&$message)
     {
         $signed = $message->getArg(Auth_OpenID_OPENID_NS, 'signed');
-        if (!$signed) {
+        if (!$signed || Auth_OpenID::isFailure($signed)) {
             // raise ValueError('Message has no signed list: %s' % (message,))
             return null;
         }
@@ -369,7 +369,7 @@ class Auth_OpenID_Association {
         $sig = $message->getArg(Auth_OpenID_OPENID_NS,
                                 'sig');
 
-        if (!$sig) {
+        if (!$sig || Auth_OpenID::isFailure($sig)) {
             return false;
         }
 
diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php
index 2ec19fc..37ae3e8 100644
--- a/Auth/OpenID/Consumer.php
+++ b/Auth/OpenID/Consumer.php
@@ -792,6 +792,9 @@ class Auth_OpenID_GenericConsumer {
 
         $signed_list_str = $message->getArg(Auth_OpenID_OPENID_NS, 'signed',
                                             Auth_OpenID_NO_DEFAULT);
+        if (Auth_OpenID::isFailure($signed_list_str)) {
+            return $signed_list_str;
+        }
         $signed_list = explode(',', $signed_list_str);
 
         $signed_fields = Auth_OpenID::addPrefix($signed_list, "openid.");
@@ -822,6 +825,10 @@ class Auth_OpenID_GenericConsumer {
         // message.
         $msg_return_to = $message->getArg(Auth_OpenID_OPENID_NS,
                                           'return_to');
+        if (Auth_OpenID::isFailure($return_to)) {
+            // XXX log me
+            return false;
+        }
 
         $return_to_parts = parse_url(Auth_OpenID_urinorm($return_to));
         $msg_return_to_parts = parse_url(Auth_OpenID_urinorm($msg_return_to));
@@ -875,6 +882,9 @@ class Auth_OpenID_GenericConsumer {
         $message = Auth_OpenID_Message::fromPostArgs($query);
         $return_to = $message->getArg(Auth_OpenID_OPENID_NS, 'return_to');
 
+        if (Auth_OpenID::isFailure($return_to)) {
+            return $return_to;
+        }
         // XXX: this should be checked by _idResCheckForFields
         if (!$return_to) {
             return new Auth_OpenID_FailureResponse(null,
@@ -925,6 +935,9 @@ class Auth_OpenID_GenericConsumer {
     {
         $assoc_handle = $message->getArg(Auth_OpenID_OPENID_NS,
                                          'assoc_handle');
+        if (Auth_OpenID::isFailure($assoc_handle)) {
+            return $assoc_handle;
+        }
 
         $assoc = $this->store->getAssociation($server_url, $assoc_handle);
 
@@ -1294,6 +1307,9 @@ class Auth_OpenID_GenericConsumer {
         $signed_list_str = $message->getArg(Auth_OpenID_OPENID_NS,
                                             'signed',
                                             Auth_OpenID_NO_DEFAULT);
+        if (Auth_OpenID::isFailure($signed_list_str)) {
+            return $signed_list_str;
+        }
         $signed_list = explode(',', $signed_list_str);
 
         foreach ($require_sigs[$message->getOpenIDNamespace()] as $field) {
@@ -1554,18 +1570,16 @@ class Auth_OpenID_GenericConsumer {
                          Auth_OpenID_OPENID_NS, 'assoc_type',
                          Auth_OpenID_NO_DEFAULT);
 
-        if ($assoc_type === null) {
-            return new Auth_OpenID_FailureResponse(null,
-              'assoc_type missing from association response');
+        if (Auth_OpenID::isFailure($assoc_type)) {
+            return $assoc_type;
         }
 
         $assoc_handle = $assoc_response->getArg(
                            Auth_OpenID_OPENID_NS, 'assoc_handle',
                            Auth_OpenID_NO_DEFAULT);
 
-        if ($assoc_handle === null) {
-            return new Auth_OpenID_FailureResponse(null,
-              'assoc_handle missing from association response');
+        if (Auth_OpenID::isFailure($assoc_handle)) {
+            return $assoc_handle;
         }
 
         // expires_in is a base-10 string. The Python parsing will
@@ -1576,14 +1590,16 @@ class Auth_OpenID_GenericConsumer {
                              Auth_OpenID_OPENID_NS, 'expires_in',
                              Auth_OpenID_NO_DEFAULT);
 
-        if ($expires_in_str === null) {
-            return new Auth_OpenID_FailureResponse(null,
-              'expires_in missing from association response');
+        if (Auth_OpenID::isFailure($expires_in_str)) {
+            return $expires_in_str;
         }
 
         $expires_in = Auth_OpenID::intval($expires_in_str);
         if ($expires_in === false) {
-            return null;
+            
+            $err = sprintf("Could not parse expires_in from association ".
+                           "response %s", print_r($assoc_response, true));
+            return new Auth_OpenID_FailureResponse(null, $err);
         }
 
         // OpenID 1 has funny association session behaviour.
@@ -1594,9 +1610,8 @@ class Auth_OpenID_GenericConsumer {
                                Auth_OpenID_OPENID2_NS, 'session_type',
                                Auth_OpenID_NO_DEFAULT);
 
-            if ($session_type === null) {
-                return new Auth_OpenID_FailureResponse(null,
-                  'session_type missing from association response');
+            if (Auth_OpenID::isFailure($session_type)) {
+                return $session_type;
             }
         }
 
@@ -1772,7 +1787,7 @@ class Auth_OpenID_AuthRequest {
      */
     function addExtensionArg($namespace, $key, $value)
     {
-        $this->message->setArg($namespace, $key, $value);
+        return $this->message->setArg($namespace, $key, $value);
     }
 
     /**
@@ -2065,6 +2080,9 @@ class Auth_OpenID_SuccessResponse extends Auth_OpenID_ConsumerResponse {
         $args = array();
 
         $msg_args = $this->message->getArgs($ns_uri);
+        if (Auth_OpenID::isFailure($msg_args)) {
+            return null;
+        }
 
         foreach ($msg_args as $key => $value) {
             if (!$this->isSigned($ns_uri, $key)) {
diff --git a/Auth/OpenID/Message.php b/Auth/OpenID/Message.php
index 74f63b0..08693ce 100644
--- a/Auth/OpenID/Message.php
+++ b/Auth/OpenID/Message.php
@@ -721,19 +721,20 @@ class Auth_OpenID_Message {
 
         if ($namespace == Auth_OpenID_OPENID_NS) {
             if ($this->_openid_ns_uri === null) {
-                // raise UndefinedOpenIDNamespace('OpenID namespace not set')
-                return null;
+                return new Auth_OpenID_FailureResponse(null,
+                    'OpenID namespace not set');
             } else {
                 $namespace = $this->_openid_ns_uri;
             }
         }
 
         if (($namespace != Auth_OpenID_BARE_NS) &&
-            (!is_string($namespace))) {
-            // raise TypeError(
-            //     "Namespace must be BARE_NS, OPENID_NS or a string. got %r"
-            //     % (namespace,))
-            return null;
+              (!is_string($namespace))) {
+            //TypeError
+            $err_msg = sprintf("Namespace must be Auth_OpenID_BARE_NS, ".
+                              "Auth_OpenID_OPENID_NS or a string. got %s",
+                              print_r($namespace, true));
+            return new Auth_OpenID_FailureResponse(null, $err_msg);
         }
 
         if (($namespace != Auth_OpenID_BARE_NS) &&
@@ -754,10 +755,11 @@ class Auth_OpenID_Message {
     function hasKey($namespace, $ns_key)
     {
         $namespace = $this->_fixNS($namespace);
-        if ($namespace !== null) {
-            return $this->args->contains(array($namespace, $ns_key));
-        } else {
+        if (Auth_OpenID::isFailure($namespace)) {
+            // XXX log me
             return false;
+        } else {
+            return $this->args->contains(array($namespace, $ns_key));
         }
     }
 
@@ -765,6 +767,9 @@ class Auth_OpenID_Message {
     {
         // Get the key for a particular namespaced argument
         $namespace = $this->_fixNS($namespace);
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
+        }
         if ($namespace == Auth_OpenID_BARE_NS) {
             return $ns_key;
         }
@@ -790,15 +795,17 @@ class Auth_OpenID_Message {
         // Get a value for a namespaced key.
         $namespace = $this->_fixNS($namespace);
 
-        if ($namespace !== null) {
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
+        } else {
             if ((!$this->args->contains(array($namespace, $key))) &&
-                ($default == Auth_OpenID_NO_DEFAULT)) {
-                return null;
+              ($default == Auth_OpenID_NO_DEFAULT)) {
+                $err_msg = sprintf("Namespace %s missing required field %s",
+                                   $namespace, $key);
+                return new Auth_OpenID_FailureResponse(null, $err_msg);
             } else {
                 return $this->args->get(array($namespace, $key), $default);
             }
-        } else {
-            return null;
         }
     }
 
@@ -807,7 +814,9 @@ class Auth_OpenID_Message {
         // Get the arguments that are defined for this namespace URI
 
         $namespace = $this->_fixNS($namespace);
-        if ($namespace !== null) {
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
+        } else {
             $stuff = array();
             foreach ($this->args->items() as $pair) {
                 list($key, $value) = $pair;
@@ -819,8 +828,6 @@ class Auth_OpenID_Message {
 
             return $stuff;
         }
-
-        return array();
     }
 
     function updateArgs($namespace, $updates)
@@ -829,13 +836,13 @@ class Auth_OpenID_Message {
 
         $namespace = $this->_fixNS($namespace);
 
-        if ($namespace !== null) {
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
+        } else {
             foreach ($updates as $k => $v) {
                 $this->setArg($namespace, $k, $v);
             }
             return true;
-        } else {
-            return false;
         }
     }
 
@@ -844,14 +851,14 @@ class Auth_OpenID_Message {
         // Set a single argument in this namespace
         $namespace = $this->_fixNS($namespace);
 
-        if ($namespace !== null) {
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
+        } else {
             $this->args->set(array($namespace, $key), $value);
             if ($namespace !== Auth_OpenID_BARE_NS) {
                 $this->namespaces->add($namespace);
             }
             return true;
-        } else {
-            return false;
         }
     }
 
@@ -859,10 +866,10 @@ class Auth_OpenID_Message {
     {
         $namespace = $this->_fixNS($namespace);
 
-        if ($namespace !== null) {
-            return $this->args->del(array($namespace, $key));
+        if (Auth_OpenID::isFailure($namespace)) {
+            return $namespace;
         } else {
-            return false;
+            return $this->args->del(array($namespace, $key));
         }
     }
 
diff --git a/Auth/OpenID/SReg.php b/Auth/OpenID/SReg.php
index 79b2cdc..6328076 100644
--- a/Auth/OpenID/SReg.php
+++ b/Auth/OpenID/SReg.php
@@ -226,7 +226,7 @@ class Auth_OpenID_SRegRequest extends Auth_OpenID_SRegBase {
         $obj->ns_uri = $obj->_getSRegNS($m);
         $args = $m->getArgs($obj->ns_uri);
 
-        if ($args === null) {
+        if ($args === null || Auth_OpenID::isFailure($args)) {
             return null;
         }
 
@@ -484,7 +484,7 @@ class Auth_OpenID_SRegResponse extends Auth_OpenID_SRegBase {
             $args = $success_response->message->getArgs($obj->ns_uri);
         }
 
-        if ($args === null) {
+        if ($args === null || Auth_OpenID::isFailure($args)) {
             return null;
         }