svn commit: r1539746 - /perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c

[st...@apache.org]

Author: stevehay
Date: Thu Nov  7 18:25:06 2013
New Revision: 1539746

URL: http://svn.apache.org/r1539746
Log:
Restructure perl_get_realm_hash() as per perl_parse_require_line() in r1539414/1539487, fixing the test for ab->cb2 and correcting the early return values (we should probably return AUTH_USER_NOT_FOUND in the case that ab->cb2 is NULL since that's currently expected behaviour, but we think AUTH_GENERAL_ERROR is wiser for now so as not to let on that we have a realm hash function; the latter is also correct in the unexpected case that ab is NULL, and will also be correct for the case that ab->cb2 is NULL in the future if PerlAddAuthnProvider ever supports an optional second handler).

Thanks to Jan Kaluža and Jeff Trawick for advice with this.

Modified:
    perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c

Modified: perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c
URL: http://svn.apache.org/viewvc/perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c?rev=1539746&r1=1539745&r2=1539746&view=diff
==============================================================================
--- perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c (original)
+++ perl/modperl/branches/httpd24threading/src/modules/perl/modperl_util.c Thu Nov  7 18:25:06 2013
@@ -1116,52 +1116,63 @@ static authn_status perl_get_realm_hash(
                                         const char *realm, char **rethash)
 {
     authn_status ret = AUTH_USER_NOT_FOUND;
-    int count;
-    SV *rh;
     const char *key;
     auth_callback *ab;
-    MP_dINTERPa(r, NULL, NULL);
 
-    if (global_authn_providers == NULL) {
-        MP_INTERP_PUTBACK(interp, aTHX);
-        return ret;
+    if (global_authn_providers == NULL ||
+        apr_hash_count(global_authn_providers) == 0)
+    {
+        return AUTH_GENERAL_ERROR;
     }
 
     key = apr_table_get(r->notes, AUTHN_PROVIDER_NAME_NOTE);
     ab = apr_hash_get(global_authn_providers, key, APR_HASH_KEY_STRING);
-    if (ab == NULL || ab->cb2) {
-        MP_INTERP_PUTBACK(interp, aTHX);
-        return ret;
+    if (ab == NULL || ab->cb2 == NULL) {
+        return AUTH_GENERAL_ERROR;
     }
 
-    rh = sv_2mortal(newSVpv("", 0));
     {
-        dSP;
-        ENTER;
-        SAVETMPS;
-        PUSHMARK(SP);
-        XPUSHs(sv_2mortal(modperl_ptr2obj(aTHX_ "Apache2::RequestRec", r)));
-        XPUSHs(sv_2mortal(newSVpv(user, 0)));
-        XPUSHs(sv_2mortal(newSVpv(realm, 0)));
-        XPUSHs(newRV_noinc(rh));
-        PUTBACK;
-        count = call_sv(ab->cb2, G_SCALAR);
-        SPAGAIN;
-
-        if (count == 1) {
-            const char *tmp = SvPV_nolen(rh);
-            ret = (authn_status) POPi;
-            if (*tmp != '\0') {
-                *rethash = apr_pstrdup(r->pool, tmp);
+        /* PerlAddAuthnProvider currently does not support an optional second
+         * handler, so ab->cb2 should always be NULL above and we will never get
+         * here. If such support is added in the future then this code will be
+         * reached. Unlike the PerlAddAuthzProvider case, the second handler here
+         * would be called during request_rec processing to obtain a password hash
+         * for the realm so there should be no problem grabbing an interpreter.
+         */
+        MP_dINTERPa(r, NULL, NULL);
+
+        {
+            SV* rh = sv_2mortal(newSVpv("", 0));
+            int count;
+            dSP;
+
+            ENTER;
+            SAVETMPS;
+            PUSHMARK(SP);
+            XPUSHs(sv_2mortal(modperl_ptr2obj(aTHX_ "Apache2::RequestRec", r)));
+            XPUSHs(sv_2mortal(newSVpv(user, 0)));
+            XPUSHs(sv_2mortal(newSVpv(realm, 0)));
+            XPUSHs(newRV_noinc(rh));
+            PUTBACK;
+            count = call_sv(ab->cb2, G_SCALAR);
+            SPAGAIN;
+
+            if (count == 1) {
+                const char *tmp = SvPV_nolen(rh);
+                ret = (authn_status) POPi;
+                if (*tmp != '\0') {
+                    *rethash = apr_pstrdup(r->pool, tmp);
+                }
             }
+
+            PUTBACK;
+            FREETMPS;
+            LEAVE;
         }
 
-        PUTBACK;
-        FREETMPS;
-        LEAVE;
+        MP_INTERP_PUTBACK(interp, aTHX);
     }
 
-    MP_INTERP_PUTBACK(interp, aTHX);
     return ret;
 }