Tatsuki Sugiura | 1 Feb 08:25 2010

Re: [PATCH] mod_fcgid: SEGV with empty output from fcgi process

Hello,

>>> In Message "Re: [PATCH] mod_fcgid: SEGV with empty output from fcgi process"
>>>            <cc67648e1001310433y67082e56nd76327314ad21bda <at> mail.gmail.com>,
>>> Jeff Trawick <trawick <at> gmail.com>  said;
> > a) is it better (and practical) to ensure that the brigade has an EOS
> > bucket to start with so this special case doesn't have to be handled

> I was tricked by the existing check for EOS; there never is one AFAICT
> (the "done" flag is the usual escape from the loop).  So your new
> check stays and the EOS check goes.

> > b) how to get a 500 error to the client in this situation (the FastCGI
> > app returns no headers)

> I still need to look at this (cgid returns 500 in this case)

Thanks for your work!

I confirmed r905031 in my environment.
It seems you fixed both already, correct?

--

-- 
Tatsuki Sugiura   mailto:sugi <at> nemui.org

Jeff Trawick | 1 Feb 13:08 2010
Picon

Re: [PATCH] mod_fcgid: SEGV with empty output from fcgi process

On Mon, Feb 1, 2010 at 2:25 AM, Tatsuki Sugiura <sugi <at> nemui.org> wrote:
> Hello,
>
>>>> In Message "Re: [PATCH] mod_fcgid: SEGV with empty output from fcgi process"
>>>>            <cc67648e1001310433y67082e56nd76327314ad21bda <at> mail.gmail.com>,
>>>> Jeff Trawick <trawick <at> gmail.com>  said;
>> > a) is it better (and practical) to ensure that the brigade has an EOS
>> > bucket to start with so this special case doesn't have to be handled
>
>> I was tricked by the existing check for EOS; there never is one AFAICT
>> (the "done" flag is the usual escape from the loop).  So your new
>> check stays and the EOS check goes.
>
>> > b) how to get a 500 error to the client in this situation (the FastCGI
>> > app returns no headers)
>
>> I still need to look at this (cgid returns 500 in this case)
>
> Thanks for your work!
>
> I confirmed r905031 in my environment.
> It seems you fixed both already, correct?

Yes; thanks again!

Jeff Trawick | 1 Feb 16:59 2010
Picon

FYI... tentatively planning to roll mod_fcgid again towards the end of the week...

... to correct a regression when used with httpd 2.0.x on some
Unix-ish platforms.

yle lists | 1 Feb 17:09 2010
Picon

[PATCH] mod_proxy sessions

Hello all,

I'd like to submit a patch (against 2.2.14) that provides mod_proxy sessions handling, that is, the ability to bind backend connections (resources) to a session-id.

When proxy sessions are enabled, the backend connections are recycled in a per-session reslist.
The worker uses a hashmap for the acquired sessions, and recycles the released ones in a reslist.

One can use the sessions to limit, per-client (provided the client can be identified, by IP or cookie or whatever), the number of connections to the backend, or to forward statefull over-http-protocols.

The session-id used for a particular request is given by the "proxy-session-id" note (r->notes), so that the other modules (or a Rewrite/SetEnv rule) can set it to a client identifier.

Another note ("proxy-session-client", true/false) can be used to bind the session to the client connection (the connection-id is the session-id), like mod_proxy 2.0.
I use it to forward client NTLM connections, since the backend connections will be per-client connection.

For the sessions to be enabled, one can use the worker parameter SessionsCache=on, and the SessionsCache(S)Max|Min parameters control the sessions reslist.
The Session(S)Max|Min parameters control the number of connections per session.

Finally, the patch also includes the functions apr_reslist_create_ex and apr_reslist_clear which would allow one to clear a reslist with a given clearor function.
I also included comments in the patch regarding the destructor of the proxy connection which, IMHO, is broken in httpd-2.2x.

Thank you for your comments,
Yann.

Attachment (httpd-2.2.14.patch): text/x-diff, 57 KiB
Graham Leggett | 1 Feb 17:48 2010

Re: [PATCH] mod_proxy sessions

On 01 Feb 2010, at 6:09 PM, yle lists wrote:

> I'd like to submit a patch (against 2.2.14) that provides mod_proxy  
> sessions handling, that is, the ability to bind backend connections  
> (resources) to a session-id.
>
> When proxy sessions are enabled, the backend connections are  
> recycled in a per-session reslist.
> The worker uses a hashmap for the acquired sessions, and recycles  
> the released ones in a reslist.
>
> One can use the sessions to limit, per-client (provided the client  
> can be identified, by IP or cookie or whatever), the number of  
> connections to the backend, or to forward statefull over-http- 
> protocols.
>
> The session-id used for a particular request is given by the "proxy- 
> session-id" note (r->notes), so that the other modules (or a Rewrite/ 
> SetEnv rule) can set it to a client identifier.
>
> Another note ("proxy-session-client", true/false) can be used to  
> bind the session to the client connection (the connection-id is the  
> session-id), like mod_proxy 2.0.
> I use it to forward client NTLM connections, since the backend  
> connections will be per-client connection.
>
> For the sessions to be enabled, one can use the worker parameter  
> SessionsCache=on, and the SessionsCache(S)Max|Min parameters control  
> the sessions reslist.
> The Session(S)Max|Min parameters control the number of connections  
> per session.
>
> Finally, the patch also includes the functions apr_reslist_create_ex  
> and apr_reslist_clear which would allow one to clear a reslist with  
> a given clearor function.
> I also included comments in the patch regarding the destructor of  
> the proxy connection which, IMHO, is broken in httpd-2.2x.

Some comments based on a quick once over.

Many small patches are easier to review than one big patch. Small  
patches are likely to be accepted quickly, leaving time for issues  
with other patches to be resolved.

Changes to APR should be packaged separately and submitted to the APR  
project, httpd and APR split from each other some time ago, and httpd  
doesn't keep it's own private changes to APR. The extensions you  
propose to reslist look interesting, and are definitely worth picking  
up there.

The patch should be to trunk rather than httpd v2.2, as all changes  
must be made to the trunk first before being considered for backport  
to v2.2.

Your changes to ap_proxy_acquire_connection() and friends changes the  
ABI of the code, and as such cannot be backported as is to v2.2 as it  
stands.

Another potential issue is that this patch extends mod_proxy instead  
of adding a new module called mod_proxy_session. mod_proxy already  
contains too much non-abstracted code, and this makes it worse.  
Ideally, the bulk of the code should be in a module of its own.

I also don't fully see why the ABI of ap_proxy_acquire_connection()  
and friends was changed, can you clarify what you were trying to do?

Regards,
Graham
--

yle | 1 Feb 18:48 2010
Picon

Re: [PATCH] mod_proxy sessions

I reply in place ...

2010/2/1 Graham Leggett <minfrin <at> sharp.fm>
On 01 Feb 2010, at 6:09 PM, yle lists wrote:

I'd like to submit a patch (against 2.2.14) that provides mod_proxy sessions handling, that is, the ability to bind backend connections (resources) to a session-id.

When proxy sessions are enabled, the backend connections are recycled in a per-session reslist.
The worker uses a hashmap for the acquired sessions, and recycles the released ones in a reslist.

One can use the sessions to limit, per-client (provided the client can be identified, by IP or cookie or whatever), the number of connections to the backend, or to forward statefull over-http-protocols.

The session-id used for a particular request is given by the "proxy-session-id" note (r->notes), so that the other modules (or a Rewrite/SetEnv rule) can set it to a client identifier.

Another note ("proxy-session-client", true/false) can be used to bind the session to the client connection (the connection-id is the session-id), like mod_proxy 2.0.
I use it to forward client NTLM connections, since the backend connections will be per-client connection.

For the sessions to be enabled, one can use the worker parameter SessionsCache=on, and the SessionsCache(S)Max|Min parameters control the sessions reslist.
The Session(S)Max|Min parameters control the number of connections per session.

Finally, the patch also includes the functions apr_reslist_create_ex and apr_reslist_clear which would allow one to clear a reslist with a given clearor function.
I also included comments in the patch regarding the destructor of the proxy connection which, IMHO, is broken in httpd-2.2x.

Some comments based on a quick once over.

Many small patches are easier to review than one big patch. Small patches are likely to be accepted quickly, leaving time for issues with other patches to be resolved.

Changes to APR should be packaged separately and submitted to the APR project, httpd and APR split from each other some time ago, and httpd doesn't keep it's own private changes to APR. The extensions you propose to reslist look interesting, and are definitely worth picking up there.

The patch should be to trunk rather than httpd v2.2, as all changes must be made to the trunk first before being considered for backport to v2.2.
 
OK, I can split the patch and adapt it to trunk if one looks interested.


Your changes to ap_proxy_acquire_connection() and friends changes the ABI of the code, and as such cannot be backported as is to v2.2 as it stands.

Cf. explanation on ap_proxy_acquire_connection() ABI change below.


Another potential issue is that this patch extends mod_proxy instead of adding a new module called mod_proxy_session. mod_proxy already contains too much non-abstracted code, and this makes it worse. Ideally, the bulk of the code should be in a module of its own.

Yes, it would be a better to use a module of its own, I have to better look at the proxy hooks to see how the proxy_session module could do the job elsewhere (most of the code would be that of the http module I guess, but I need to look closer).
My first goal was to make it work ...


I also don't fully see why the ABI of ap_proxy_acquire_connection() and friends was changed, can you clarify what you were trying to do?
 
I wanted ap_proxy_acquire_connection() be able to access the r->notes and r->connection (for the session-id/client to be dynamically set-able), the server record was of no help for me.

Maybe with a separate module I could use a new ap_proxy_acquire_session() instead, let me think a bit on that.


Regards,
Graham
--

Thank you for your comments,
regards,
Yann.
Graham Leggett | 1 Feb 19:39 2010

Re: [PATCH] mod_proxy sessions

On 01 Feb 2010, at 7:48 PM, yle wrote:

> Yes, it would be a better to use a module of its own, I have to  
> better look at the proxy hooks to see how the proxy_session module  
> could do the job elsewhere (most of the code would be that of the  
> http module I guess, but I need to look closer).
> My first goal was to make it work ...

Makes sense, you have to start somewhere.

> I wanted ap_proxy_acquire_connection() be able to access the r- 
> >notes and r->connection (for the session-id/client to be  
> dynamically set-able), the server record was of no help for me.

I recall running into a similar dilemma a while back, I think the  
change from server_rec to request_rec does make sense.

Such a change would only be possible on trunk though, as the ABI for  
2.2 is cast in stone at this point. Trunk has started being released  
as an alpha version, so a potential server_rec to request_rec change  
should see the light of day quite soon.

I suggest breaking the patch down into at the least:

- The APR changes, to be submitted to the APR project as a unit.

- The server_rec -> request_rec change, which can go in on its own.

- The rest can follow.

Regards,
Graham
--

Chris Darroch | 1 Feb 20:21 2010

Re: FYI... tentatively planning to roll mod_fcgid again towards the end of the week...

Jeff Trawick wrote:

> ... to correct a regression when used with httpd 2.0.x on some
> Unix-ish platforms.

   OK, noted.  I very much hope to pack in some additional changes
to the wrapper management code this week; time's still on my side
at the moment.  I spent some of Friday on it but what seemed like it
might be a one- or two-liner revealed deeper problems that'll take
a bigger fix than I'd initially hoped.  I'll keep you posted as
the week progresses if commits from me aren't forthcoming.  Thanks,

Chris.

--

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

Stefan Fritsch | 1 Feb 21:14 2010
Picon

Re: mod_dav inconsistent behaviour for GET requests

On Sunday 31 January 2010, Justin Erenkrantz wrote:
> On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch <sf <at> sfritsch.de> 
wrote:
> > On Sat, 30 Jan 2010, Justin Erenkrantz wrote:
> >> I don't see how your patch would "intentionally" break - the
> >> failure mechanism is that the source scripts are served - not
> >> that a configuration error stops the server from running.  --
> >> justin
> >
> > Surely a fatal server error is not a necessary condition for
> > something to be broken?
> 
> When it has the capability of exposing source that would not
>  otherwise be served, absolutely.
> 
> The fundamental flaw with this patch is that dav_fixups runs after
> core_override_type - so the "none" handler simply won't get
>  reassigned by the rest of the applicable configs - ie set to CGI
>  or PHP or whatnot.  So, it would simply fall through and go to the
>  default handler.  Ouch.  -- justin

That's exactly what the patch is supposed to do. Therefore I would not 
call it flawed.

I think that the auth changes from 2.2 to trunk are so large that 
anyone upgrading will have to carefully review and test his 
configuration for security problems anyway. An additional change in 
the behaviour of mod_dav wouldn't create much of an additional problem 
(if it is documented correctly).

But since quite a few people disagree with me here, an alternative 
could be an additional directive (or second argument to 'Dav') that 
allows to configure the behaviour. For example

DavHandleMethods all
DavHandleMethods exceptPOST
DavHandleMethods exceptGET,POST

For 2.4, one could then leave the default at exceptGET,POST / 
exceptPOST (depending on the dav provider), just like it is for 2.2.x. 
But if the user does not specify DavHandleMethods explicitly, httpd 
could log a notice saying:

"DavHandleMethods defaulting to 'exceptGET,POST'. The default will 
change to 'all' with the next major release of httpd. Please specify 
DavHandleMethods explicitly."

Or one could even make the new directive mandatory, with httpd 
refusing to start without it.

Would this address your concerns?

Cheers,
Stefan

Christian Folini | 1 Feb 21:59 2010

Re: [PATCH] Logging the handler in the access log

On Mon, Feb 01, 2010 at 01:20:21AM +0200, Graham Leggett wrote:
> Definitely sounds good in principle.

thanks.

> Would it be possible to update the 
> documentation for this as well? It involves updating the XML files in the 
> documentation tree.

Sure. Here you go:

/data/svn/apache-2.2.x-docs/manual/mod>svn diff
Index: mod_log_config.xml
===================================================================
--- mod_log_config.xml	(revision 905403)
+++ mod_log_config.xml	(working copy)
 <at>  <at>  -159,6 +159,9  <at>  <at> 
     <tr><td><code>%r</code></td>
         <td>First line of request</td></tr>

+    <tr><td><code>%R</code></td>
+        <td>The handler generating the response (if any).</td></tr>
+
     <tr><td><code>%s</code></td>
         <td>Status. For requests that got internally redirected, this is
         the status of the *original* request --- <code>%&gt;s</code>
 <at>  <at>  -267,6 +270,11  <at>  <at> 
       format provided by <module>mod_logio</module> will log the
       actual number of bytes sent over the network.</p>

+      <p><code class="module"><a href="../mod/mod_cache.html">Mod_cache</a></code> 
+      is implemented as a quick-handler and not as a standard handler.
+      Therefore, the <code>%R</code> format string will not return 
+      any handler information when content caching is involved.</p>
+
     </section>

     <section id="examples"><title>Examples</title>

I think the remark about mod_cache is necessary as a caveat.

(Hints on how I could extend my patch to log mod_cache too
are welcome.)

regs,

Christian

--

-- 
The past is the father of the present.
--- Hercule Poirot in Agatha Christie's "Halloween Party"


Gmane