Making http_auth_request_module a first-class citizen? [patch]

[Maxim Dounin]

Hello!

On Sun, Feb 19, 2012 at 05:12:17AM +0400, Max wrote:

> 
> 17 февраля 2012, 15:19 от Maxim Dounin <mdounin at mdounin.ru>:
> > On Fri, Feb 17, 2012 at 12:28:14PM +0400, Max wrote:
> > > Maxim, you haven't even taken a look at my patch, have you? Because
> > > if you had, you wouldn't have made such unsubstantiated claims.
> > 
> > I have, despite the fact that it was provided as a link only.
> 
> I usually send in patches on postcards, but I had run out of postcards.
> I'm glad this Internet thing seems to work, although your replies seem
> to indicate your download request had header_only set to 1. :-P

Posting patches in the message itself makes review much easier.  
And you may also want to fix your email client to don't use html 
escaping in text mesages.

> > I stand corrected. Your patch broke only $request variable, not
> > the $request_method (which always comes from the main request).
> 
> You're wrong, my patch did not break anything. Go ahead and put
> any variables in those /private/ and /auth location blocks and
> see for yourself. I know it must be difficult to decipher, but
> could you try to guess what the following part of my patch does?

It looks like you don't understand how variables work.  Try the 
following without your patch and with it:

    log_format test "request: $request";
    access_log /path/to/log test;

    location / {
        auth_request /auth;
    }

    location = /auth {
        proxy_pass http://some_auth_backend;
        proxy_set_header X-Original-Request $request;
    }

[...]

> location /private/ {
>     auth_request /auth;
>     set $request_private $request;

This will calculate $request and cache it forever.  As this will 
happen in main request context - everything will be good, i.e. 
original request line will be used everywhere.

On the other hand, if $request calculation will happen in auth 
subrequest with your patch - the modified request line will be 
cached for the rest of request processing.

[...]

> > On the other hand, setting method/request line increase comlexity
> > and overhead in normal case, as well as subject to bugs (at least
> > two were identified above).
> 
> Are you serious? You're comparing the complexity and overhead of
> completely in-memory operations (memory allocation, assignment,
> request line scanning and two memcpy() calls) on a few dozen
> bytes to reading from disk, processing, buffering and sending
> hundreds of kilobytes on a closed connection?

It's sad you don't understand what I wrote.  In the normal case 
there are *no* overhead for extra bytes sent, but there *is* 
overhead for extra processing.  But it doesn't really matter 
though, it's complexity (and bugs) which really matters.

And, just to make things more clear, complexity is about code 
complexity which makes maintanance, debugging and further 
modifications harder.

[...]

Maxim Dounin