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; }