Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-07 Thread Graham Leggett

On 06 Nov 2010, at 10:32 PM, Stefan Fritsch wrote:

I think I have made my intentions clear in my first mail from  
October 23rd. But maybe I should have mentioned it also in the later  
mails.


The grammar and in particular the string handling in the SSI  
expression parser is so weird that it would be very difficult to  
handle it in a backward compatible way with the same parser as the  
ssl_expr grammar. At the minimum, one would need to rewrite the  
tokenizer. Therefore I thought it a much more sensible approach to  
handle the SSI expression compatibility by just putting that parser  
back into mod_include. So the total number of parsers in httpd has  
not changed by my work, but we now have the better parser as  
univeral parser.


Of course it would be trivial to offer the new ap_expr as an  
alternative in mod_include. Either by allowing to select the parser  
with a config directive or by an alternative syntax in the SSI  
document, like
'!--#if apexpr=...--'. Does this sound reasonable? Which of the  
two alternatives would you prefer? It may also be possible to at  
least make the generalized variable lookup available in the old SSI  
parser.


If we can offer a toggle switch directive, and declare the old syntax  
as deprecated (and remove it in v2.6/v3.0), I think it would be ideal.


BTW, there are a number of other places where I want to allow  
ap_expr as an alternative to the current syntax:


- SetEnvIf (maybe as SetEnvIf expr=...?)
- Conditional access log (simply by using expr=... instead of  
env=...)

- maybe RewriteCond


+1.

Regards,
Graham
--



Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-06 Thread Graham Leggett

On 06 Nov 2010, at 4:03 PM, s...@apache.org wrote:


Author: sf
Date: Sat Nov  6 14:03:13 2010
New Revision: 1032059

URL: http://svn.apache.org/viewvc?rev=1032059view=rev
Log:
Put the expression parser back into mod_include
This reverts r642559 and r642978



-#include ap_expr.h


I don't follow, I was understanding that mod_include and mod_ssl would  
now use the same parser?


Regards,
Graham
--



Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-06 Thread Nick Kew

On 6 Nov 2010, at 14:03, s...@apache.org wrote:

 Author: sf
 Date: Sat Nov  6 14:03:13 2010
 New Revision: 1032059
 
 URL: http://svn.apache.org/viewvc?rev=1032059view=rev
 Log:
 Put the expression parser back into mod_include

Hang on!

Surely we don't have to admit defeat in using a semi-universal expression 
parser?
(I say semi, because mod_rewrite is probably too hard to convert).

-- 
Nick Kew

Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-06 Thread Stefan Fritsch

On Sat, 6 Nov 2010, Nick Kew wrote:

URL: http://svn.apache.org/viewvc?rev=1032059view=rev
Log:
Put the expression parser back into mod_include


Hang on!

Surely we don't have to admit defeat in using a semi-universal expression 
parser?
(I say semi, because mod_rewrite is probably too hard to convert).


I think I have made my intentions clear in my first mail from October 
23rd. But maybe I should have mentioned it also in the later mails.


The grammar and in particular the string handling in the SSI expression 
parser is so weird that it would be very difficult to handle it in a 
backward compatible way with the same parser as the ssl_expr grammar. At 
the minimum, one would need to rewrite the tokenizer. Therefore I thought 
it a much more sensible approach to handle the SSI expression 
compatibility by just putting that parser back into mod_include. So the 
total number of parsers in httpd has not changed by my work, but we now 
have the better parser as univeral parser.


Of course it would be trivial to offer the new ap_expr as an alternative 
in mod_include. Either by allowing to select the parser with a config 
directive or by an alternative syntax in the SSI document, like
'!--#if apexpr=...--'. Does this sound reasonable? Which of the two 
alternatives would you prefer? It may also be possible to at least make 
the generalized variable lookup available in the old SSI parser.


BTW, there are a number of other places where I want to allow ap_expr as 
an alternative to the current syntax:


- SetEnvIf (maybe as SetEnvIf expr=...?)
- Conditional access log (simply by using expr=... instead of env=...)
- maybe RewriteCond