[PATCH] Query string parsing for mod_include.c

[fr...@friedo.com]

Attached is a patch that impliments a new directive in mod_include.c,
#parseqs, which parses the name=value pairs on the query string and places
the results in the subprocess environment table, allowing you to do highly
neat things with SSI scripts. I found this feature to be quite useful.

Caveats:
1. A potential security problem is that a malicious person may be able to
use it to override important environment variables on a page where he
knows #parseqs is in use. We might want to disallow all-CAPS variables
from being modified in this fashion.

2. There is no support for query string variables with multiple values.


Please go easy on me. :) I haven't submitted anything to Apache before,
but I found this hack to be quite useful. I look forward to your comments.

Re: [PATCH] Query string parsing for mod_include.c

[Brian Akins <ba...@web.turner.com>]

On Tue, 2003-09-09 at 03:00, friedo@friedo.com wrote:
> Attached is a patch that impliments a new directive in mod_include.c,
> #parseqs, 

Couldn't this be better implemented using
"APR_OPTIONAL_FN_TYPE(ap_register_include_handler)" type things in a
separate module.  This seems to be a much better way to extend SSI
without mucking with mod_include.  I have done some custom directives
doing this and it just seems "cleaner."

-- 
Brian Akins <ba...@web.turner.com>
CNN Internet Technologies

Re: [PATCH] Query string parsing for mod_include.c

[fr...@friedo.com]

Hi, Andre. Thanks for your feedback. I will definately port this to the
2.1 branch and submit a new patch.

> - one should recognize ; as delimiter as well (ok, trivial)

Easy enough.

> - to circumvent the security flaw, I'd suggest to extend the #set
> handler
>   instead, for example:
>     <!--#set var="foo" query="param_name" -->, which would be really
> safe.
>   I'm not sure, whether the query parameter should be expanded.
> Opinions?

Another idea I thought of was to put the query string vars in a seperate
table, and have a special prefix for accessing that table, (something like
@var instead of $var.) That would prevent overwriting important stuff in
subprocess_env. I also like your idea, though it is a bit more cumbersome
for the person writing the SSI.

> - The second one could be solved with things like
>   <!#--set var="foo" query="param_name[i]" -->, where i starts with 0 or
> 1 (?).

It should start with zero of course. :P

Re: [PATCH] Query string parsing for mod_include.c

[André Malo <nd...@perlig.de>]

friedo@friedo.com wrote:

>Attached is a patch that impliments a new directive in mod_include.c,
>#parseqs, which parses the name=value pairs on the query string and places
>the results in the subprocess environment table, allowing you to do highly
>neat things with SSI scripts. I found this feature to be quite useful.
>
>Caveats:
>1. A potential security problem is that a malicious person may be able to
>use it to override important environment variables on a page where he
>knows #parseqs is in use. We might want to disallow all-CAPS variables
>from being modified in this fashion.
>
>2. There is no support for query string variables with multiple values.
>
>
In general this is really a desired feature. Thanks!
There are some issues, anyway ;-)

- one should recognize ; as delimiter as well (ok, trivial)

- to circumvent the security flaw, I'd suggest to extend the #set handler
  instead, for example:
    <!--#set var="foo" query="param_name" -->, which would be really safe.
  I'm not sure, whether the query parameter should be expanded. Opinions?

- The second one could be solved with things like
  <!#--set var="foo" query="param_name[i]" -->, where i starts with 0 or 
1 (?).

- what about unescaping the values?

There's still another point. I'd add this to the 2.1 branch only, 
because (a) we're going forward *there*, (b) mod_include was redesigned 
to give a way better interface and (c) we should not play with our (not 
really, but ...) stable branch.

>Please go easy on me. :) I haven't submitted anything to Apache before,
>but I found this hack to be quite useful. I look forward to your comments.
>
Please attach patches as text/plain.

Thanks, nd