httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r646285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_auth_form.xml modules/aaa/config.m4 modules/aaa/mod_auth_form.c
Date Thu, 01 May 2008 22:01:01 GMT
Ruediger Pluem wrote:

>> +        +//        if (APR_SUCCESS == 
>> apr_password_validate(apr_pstrcat(r->pool, sent_user, ":", site, NULL),
>> +//                                                 sent_hash)) {
> 
> This is a C++ style comment that breaks with ANSI compilers.
> I think it would be better to remove the code or use a #if 0 #endif block.

This shouldn't even be here at all.

>> +    if (HTTP_UNAUTHORIZED == rv && !conf->disable_no_store) {
>> +        apr_table_addn(r->headers_out, "Cache-Control", "no-store");
>> +        apr_table_addn(r->err_headers_out, "Cache-Control", "no-store");
> 
> Isn't it sufficient to add this r->err_headers_out?

When I tried it wasn't, no. I will have to dig to see why.

>> +                apr_table_set(r->headers_out, "Location", sent_loc);
>> +                return HTTP_MOVED_PERMANENTLY;
> 
> Shouldn't this be HTTP_TEMPORARY_REDIRECT?

Not sure, can you explain why it would be temporary?

>> +    /* return the underlying error, or OK on success */
>> +    return r->status == HTTP_OK || r->status == OK ? OK : r->status;
> 
> Why not returning r->status here directly?

Because when it did as I recall, the HTTP_OK value triggered the error 
document handler, which broke the request.

Regards,
Graham
--


Mime
View raw message