You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2007/05/24 01:39:48 UTC

[vote] Piped loggers and APR_SHELLCMD_ENV

While I'm working on a solution to permit cmd.exe to be launched from
a service process within Win32, I'm still struck by the inefficiency
here and feel we need to resolve the core issue.

To remind you - Revision 104019 modified server/log.c to launch the
piped logger process under a shell.  Previously the system would parse
ErrorLog "|logsorter"  as  httpd -> logsorter, after this patch it
resolves as httpd -> sh -> logsorter.  80% of the time this simply
wasn't necessary, but certainly it's good for a small percentage of
users.  This became effective as of 2.0.51, prior to that version the
2.0.50 and earlier releases did not create this extraneous sh process.

So I brought up to the list 'fixing' this with an additional meta
character to follow | that would distinguish sh from non-sh invocations,
and permit both.  Since we really need to fix this in 2.2 / 2.0 and want
to do so harming as few users as possible (it would be clearly called
out in CHANGES, but none the less) please vote between one of these
two possible solutions that could be applied to 2.0 and 2.2 branches...

 [ ] Revert to "|foo" to invoke foo, and
     add "|$foo" syntax to launch foo via sh

 [ ] Retain "|foo" to invoke foo through sh, and
     add "||foo" syntax to directly launch foo

These were the best solutions I could come up with, others are welcome.

Bill

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> On Tue, May 29, 2007 at 06:36:07PM -0500, William Rowe wrote:
>> I'm retracting my two proposed choices and going with Option #3 :)  Does
>> anyone object to Jeff's weird proposal below?  I think it's the best of
>> both worlds.
> 
> *Guessing* whether the user wants to use a shell based on some syntactic 
> analysis of the command line sounds awful.  "When I renamed my logrotate 
> command to 'super|logger' why does the server start behaving like 
> <xyz>?"  "Why does my server only start doing <abc> when I use "| cat" 
> in the piped logger command?"
> 
> Having a specific syntax like your "||" proposal to allow users to 
> explicitly avoid a shell is one thing: otherwise, again: why should 
> httpd grow lots of fugly code to work around an APR bug?

Perhaps you missed the bottom line - both || and |$ would be grokked.
No ambiguity, second guessed only for non-explicit cases.  Most folks
don't NEED the shell in the way - but Jeff's patch late in 2.0 added
this as a feature for those who would like to "|logresolve|rotatelogs"
or similar, the edge case.

And this IS NOT strictly an APR bug, it's the waste of one, or thousands
(in the case of a mass vhost) useless shells running on the system.

So I personally like two separate, explicit commands, and the option to
pick the right flavor.

Tell me a platform that any sane admin will include a pipe symbol in the
name of the file.  Win/OS kernels don't allow it.  Unix only tolerates
it when it's escaped.  Your example above is 'super\|logger' maybe.

Three pipe symbols to test, and only on startup.  I don't see the pain?


Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Joe Orton <jo...@redhat.com>.
On Tue, May 29, 2007 at 06:36:07PM -0500, William Rowe wrote:
> I'm retracting my two proposed choices and going with Option #3 :)  Does
> anyone object to Jeff's weird proposal below?  I think it's the best of
> both worlds.

*Guessing* whether the user wants to use a shell based on some syntactic 
analysis of the command line sounds awful.  "When I renamed my logrotate 
command to 'super|logger' why does the server start behaving like 
<xyz>?"  "Why does my server only start doing <abc> when I use "| cat" 
in the piped logger command?"

Having a specific syntax like your "||" proposal to allow users to 
explicitly avoid a shell is one thing: otherwise, again: why should 
httpd grow lots of fugly code to work around an APR bug?

joe

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
I'm retracting my two proposed choices and going with Option #3 :)  Does
anyone object to Jeff's weird proposal below?  I think it's the best of
both worlds.

Speak up before I hack this in.

Bill

William A. Rowe, Jr. wrote:
> Jeff Trawick wrote:
>> On 5/23/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
>>> While I'm working on a solution to permit cmd.exe to be launched from
>>> a service process within Win32, I'm still struck by the inefficiency
>>> here and feel we need to resolve the core issue.
>> Apparently it is a good APR testcase as well ;)
> 
> :)  No argument - I don't want to lose the 'feature', simply offer both
> behaviors...
> 
>>>  [ ] Revert to "|foo" to invoke foo, and
>>>      add "|$foo" syntax to launch foo via sh
>>>
>>>  [ ] Retain "|foo" to invoke foo through sh, and
>>>      add "||foo" syntax to directly launch foo

>> Just to be weird:
>>
>> |$foo syntax launches foo via sh
>> ||foo syntax launches foo directly
>>
>> |foo tries to make the right decision:
> 
> ++1 - I prefer your idea, if everyone else can buy into it!
> 
>> Windows:
>> APR is busted, so launch foo directly
> 
> or rather, once cmd.exe behaves in the service process, we can fall over
> to the unix rules...
> 
>> Other platforms:
>>
>> starts with slash and contains no redirection?
>>  launches foo directly
>> else
>>  launches foo via sh
>>
>> This is expected to do the right thing for just about everybody --
>> almost no regression, with neatness improvement (Unix users not
>> needing to see the extra /bin/sh hanging around) or functional
>> improvement (Windows users avoiding APR problem) for most folks.
> 
> Nice
> 
>> Drawback: Some people may not be ready to understand the resulting doc.
> 
> True - but with your solution, for the most part, folks won't need to.
> The ones who actually want to have the fine control will take the time
> to read it three times and figure it out.
> 
> 


Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> On 5/23/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
>> While I'm working on a solution to permit cmd.exe to be launched from
>> a service process within Win32, I'm still struck by the inefficiency
>> here and feel we need to resolve the core issue.
> 
> Apparently it is a good APR testcase as well ;)

:)  No argument - I don't want to lose the 'feature', simply offer both
behaviors...

>>  [ ] Revert to "|foo" to invoke foo, and
>>      add "|$foo" syntax to launch foo via sh
>>
>>  [ ] Retain "|foo" to invoke foo through sh, and
>>      add "||foo" syntax to directly launch foo
> 
> Just to be weird:
> 
> |$foo syntax launches foo via sh
> ||foo syntax launches foo directly
> 
> |foo tries to make the right decision:

++1 - I prefer your idea, if everyone else can buy into it!

> Windows:
> APR is busted, so launch foo directly

or rather, once cmd.exe behaves in the service process, we can fall over
to the unix rules...

> Other platforms:
> 
> starts with slash and contains no redirection?
>  launches foo directly
> else
>  launches foo via sh
> 
> This is expected to do the right thing for just about everybody --
> almost no regression, with neatness improvement (Unix users not
> needing to see the extra /bin/sh hanging around) or functional
> improvement (Windows users avoiding APR problem) for most folks.

Nice

> Drawback: Some people may not be ready to understand the resulting doc.

True - but with your solution, for the most part, folks won't need to.
The ones who actually want to have the fine control will take the time
to read it three times and figure it out.

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Jeff Trawick <tr...@gmail.com>.
On 5/23/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> While I'm working on a solution to permit cmd.exe to be launched from
> a service process within Win32, I'm still struck by the inefficiency
> here and feel we need to resolve the core issue.

Regarding the inefficiency, it doesn't seem to be a big deal.  It
takes a trivial amount longer to start up the piped logger, and a
process hangs around consuming a trivial amount of resources.

I think it is more an issue of neatness -- does somebody want to see
that process hanging around.

Apparently it is a good APR testcase as well ;)

> So I brought up to the list 'fixing' this with an additional meta
> character to follow | that would distinguish sh from non-sh invocations,
> and permit both.  Since we really need to fix this in 2.2 / 2.0 and want
> to do so harming as few users as possible (it would be clearly called
> out in CHANGES, but none the less) please vote between one of these
> two possible solutions that could be applied to 2.0 and 2.2 branches...
>
>  [ ] Revert to "|foo" to invoke foo, and
>      add "|$foo" syntax to launch foo via sh
>
>  [ ] Retain "|foo" to invoke foo through sh, and
>      add "||foo" syntax to directly launch foo

Just to be weird:

|$foo syntax launches foo via sh
||foo syntax launches foo directly

|foo tries to make the right decision:

Windows:
APR is busted, so launch foo directly

Other platforms:

starts with slash and contains no redirection?
  launches foo directly
else
  launches foo via sh

This is expected to do the right thing for just about everybody --
almost no regression, with neatness improvement (Unix users not
needing to see the extra /bin/sh hanging around) or functional
improvement (Windows users avoiding APR problem) for most folks.

Drawback: Some people may not be ready to understand the resulting doc.

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Jim Jagielski <ji...@jaguNET.com>.
On May 24, 2007, at 4:04 AM, Sander Temme wrote:

>
> On May 23, 2007, at 4:39 PM, William A. Rowe, Jr. wrote:
>
>>  [ ] Revert to "|foo" to invoke foo, and
>>      add "|$foo" syntax to launch foo via sh
>
> I like this one the best, since it consumes fewest resources in the  
> default case.
>
>>  [ ] Retain "|foo" to invoke foo through sh, and
>>      add "||foo" syntax to directly launch foo
>>
>> These were the best solutions I could come up with, others are  
>> welcome.
>
> However, the indirection through the shell is now the default case  
> and has been for quite a while... I figure dropping it now might  
> violate the principle of least astonishment.
>
> I prefer the first one, but given the circumstances see myself  
> tending towards the second alternative.
>

+1...


Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Sander Temme <sa...@temme.net>.
On May 23, 2007, at 4:39 PM, William A. Rowe, Jr. wrote:

>  [ ] Revert to "|foo" to invoke foo, and
>      add "|$foo" syntax to launch foo via sh

I like this one the best, since it consumes fewest resources in the  
default case.

>  [ ] Retain "|foo" to invoke foo through sh, and
>      add "||foo" syntax to directly launch foo
>
> These were the best solutions I could come up with, others are  
> welcome.

However, the indirection through the shell is now the default case  
and has been for quite a while... I figure dropping it now might  
violate the principle of least astonishment.

I prefer the first one, but given the circumstances see myself  
tending towards the second alternative.

S.


-- 
sander@temme.net              http://www.temme.net/sander/
PGP FP: 51B4 8727 466A 0BC3 69F4  B7B8 B2BE BC40 1529 24AF


Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Ruediger Pluem <rp...@apache.org>.

On 05/24/2007 11:27 AM, Joe Orton wrote:
> On Wed, May 23, 2007 at 06:39:48PM -0500, William Rowe wrote:
> 
>>While I'm working on a solution to permit cmd.exe to be launched from
>>a service process within Win32, I'm still struck by the inefficiency
>>here and feel we need to resolve the core issue.
> 
> ...
> 
>> [ ] Revert to "|foo" to invoke foo, and
>>     add "|$foo" syntax to launch foo via sh
> 
> 
> This would be a regression, it will break working configurations.
> 
> 
>> [ ] Retain "|foo" to invoke foo through sh, and
>>     add "||foo" syntax to directly launch foo
> 
> 
> No strong objection to doing this, but it seems like a case of adding 
> config foo to work around an APR bug; though I guess there's a 
> reasonable argument that it's a useful feature regardless.
> 
> Is the APR bug really so intractable?

I agree with Joe here. As far as I remember this is mainly a windows issue, correct?
Can't this be fixed somehow in APR?

If not, I would vote for the second option to avoid regressions in existing configurations.

Regards

Rüdiger



Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 23, 2007 at 06:39:48PM -0500, William Rowe wrote:
> While I'm working on a solution to permit cmd.exe to be launched from
> a service process within Win32, I'm still struck by the inefficiency
> here and feel we need to resolve the core issue.
...
>  [ ] Revert to "|foo" to invoke foo, and
>      add "|$foo" syntax to launch foo via sh

This would be a regression, it will break working configurations.

>  [ ] Retain "|foo" to invoke foo through sh, and
>      add "||foo" syntax to directly launch foo

No strong objection to doing this, but it seems like a case of adding 
config foo to work around an APR bug; though I guess there's a 
reasonable argument that it's a useful feature regardless.

Is the APR bug really so intractable?

Regards,

joe

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
William A. Rowe, Jr. wrote:
> While I'm working on a solution to permit cmd.exe to be launched from
> a service process within Win32, I'm still struck by the inefficiency
> here and feel we need to resolve the core issue.

FYI - Paul asked on httpd about the discussion;  see

http://mail-archives.apache.org/mod_mbox/httpd-dev/200701.mbox/browser

subject "Piped logger nightmares" for the original thread.

Bill

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Ruediger Pluem <rp...@apache.org>.

On 05/24/2007 02:39 PM, Colm MacCarthaigh wrote:
> On Wed, May 23, 2007 at 06:39:48PM -0500, William A. Rowe, Jr. wrote:
> 
>>So I brought up to the list 'fixing' this with an additional meta
>>character to follow | that would distinguish sh from non-sh invocations,
>>and permit both.
> 
> 
> Wouldn't "| exec logger" work?

On Windows?

Regards

Rüdiger

Re: [vote] Piped loggers and APR_SHELLCMD_ENV

Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, May 23, 2007 at 06:39:48PM -0500, William A. Rowe, Jr. wrote:
> So I brought up to the list 'fixing' this with an additional meta
> character to follow | that would distinguish sh from non-sh invocations,
> and permit both.

Wouldn't "| exec logger" work?

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net