svn commit: r573831

Hi,

sorry I can't reply directly to this thread  
http://mail-archives.apache.or
g/mod_mbox/httpd-dev/200709.mbox/%3c46E3BFE1.9050004apache.org%3e  
- i only (re)subscribed to this list today after I found the
thread.

> +/*
> + * Escapes a uri in a similar way as php's urlencode
does.
> + * Based on ap_os_escape_path in server/util.c
> + */
> +static char *escape_uri(apr_pool_t *p, const char
*path) {
> +    char *copy = apr_palloc(p, 3 * strlen(path) + 3);
> +    const unsigned char *s = (const unsigned char
*)path;
> +    unsigned char *d = (unsigned char *)copy;
> +    unsigned c;
> +
> +    while ((c = *s)) {
> +        if (apr_isalnum(c) || c == '_') {
> +            *d++ = c;
> +        }
> +        else if (c == ' ') {
> +            *d++ = '+';
> +        }
> +        else {
> +            d = c2x(c, '%', d);
> +        }
> +        ++s;
> +    }
> +    *d = '';
> +    return copy;
> +}
> +
Ruediger Pluem:
> Does it make sense to duplicate code? Shouldn't this be
placed in util.c?
It most likely should. I placed it there because I thought
the patch would  
get a higher acceptance if it just touched mod_rewrite.

> +                    /* escape the backreference */
> +                    char *tmp2, *tmp;
> +                    tmp = apr_palloc(pool, span + 1);
> +                    strncpy(tmp, bri->source +
bri->regmatch[n].rm_so,  
> span);
Ruediger Pluem:
> How about using apr_pstrndup instead?
Yes, I was not aware of that function.


André Malo:
> This spreads another uri escaper copy around. Why can't
we take 
> ap_escape_uri? Without deep digging: what's the
difference?
>Also I don't like the ' ' => '+' transition, which
should not be applied  
> forpaths. It's safer to translate that always to %20, I
guess.
The main difference is this escaping of ' ' to '+'. The
reason for this is  
that while ' ' gets encoded as %20 in paths, it gets encoded
as '+' in  
query strings (afaik for historic reasons). Therefore,
languages which  
interpret the query string (like PHP) might get confused if
they receive a  
%20 in the query string (or at least that was my concern).


André Malo:
> By the way, I'm wondering why nobody picked up the
suggested use of the 
> escape rewrite map (or I overread it).
I don't know if that works but I developed the patch because
it can be  
used in a directory-context and the RewriteMap can't.

I've created another patch, hopefully you'll like it
better.

Regards,
Guenther

On Mon, 22 Oct 2007 21:03:25 +0200
Günther Gsenger <guenther.gsengergmail.com> wrote:

> Hi,

Hi, thanks for revisiting this!

> Ruediger Pluem:
> > Does it make sense to duplicate code? Shouldn't
this be placed in
> > util.c?
> It most likely should. I placed it there because I
thought the patch
> would get a higher acceptance if it just touched
mod_rewrite.

I think perhaps Rudiger's point is that the patch
essentially
duplicates ap_escape_path_segment, except for ' ' -->
'+'.

> André Malo:
> > This spreads another uri escaper copy around. Why
can't we take 
> > ap_escape_uri? Without deep digging: what's the
difference?
> >Also I don't like the ' ' => '+' transition,
which should not be
> >applied  
> > forpaths. It's safer to translate that always to
%20, I guess.
> The main difference is this escaping of ' ' to '+'. The
reason for
> this is that while ' ' gets encoded as %20 in paths, it
gets encoded
> as '+' in query strings (afaik for historic reasons).
Therefore,
> languages which interpret the query string (like PHP)
might get
> confused if they receive a %20 in the query string (or
at least that
> was my concern).

That sounds plausible, but I'm not sure.  Anyone else?

> I've created another patch, hopefully you'll like it
better.

Looks good, but it might be nice to avoid introducing
another
new escape function.  If space vs + is the issue, perhaps
we
could fix it by making the third arg to ap_os_escape_path
an
enum, with an option to escape space --> +.

Just thinking out loud.  I won't apply anything yet, but
please
bug me if it's still languishing here by the weekend.

-- 
Nick Kew

Application Development with Apache - the Apache Modules
Book
http://www.apachetutor.or
g/

Nick Kew wrote:
> Günther Gsenger <guenther.gsengergmail.com> wrote:
>> André Malo:
>>> This spreads another uri escaper copy around.
Why can't we take 
>>> ap_escape_uri? Without deep digging: what's the
difference?
>>> Also I don't like the ' ' => '+' transition,
which should not be
>>> applied  
>>> forpaths. It's safer to translate that always
to %20, I guess.
>> The main difference is this escaping of ' ' to '+'.
The reason for
>> this is that while ' ' gets encoded as %20 in
paths, it gets encoded
>> as '+' in query strings (afaik for historic
reasons). Therefore,
>> languages which interpret the query string (like
PHP) might get
>> confused if they receive a %20 in the query string
(or at least that
>> was my concern).
> 
> That sounds plausible, but I'm not sure.  Anyone else?

I strongly expect every CGI consumer expects to see '+'
where appropriate,
and changing this will alter a vast number of CGI scripts. 
Not all of them
broken, of course, but some will be.

You can't touch QUERY_ARGS escaping, as it's well defined. 
If the patch
does this it needs refinement.

Bill

* NICK KEW WROTE:

> > THE MAIN DIFFERENCE IS THIS ESCAPING OF ' ' TO
'+'. THE REASON FOR
> > THIS IS THAT WHILE ' ' GETS ENCODED AS %20 IN
PATHS, IT GETS ENCODED
> > AS '+' IN QUERY STRINGS (AFAIK FOR HISTORIC
REASONS). THEREFORE,
> > LANGUAGES WHICH INTERPRET THE QUERY STRING (LIKE
PHP) MIGHT GET
> > CONFUSED IF THEY RECEIVE A %20 IN THE QUERY STRING
(OR AT LEAST THAT
> > WAS MY CONCERN).
>
> THAT SOUNDS PLAUSIBLE, BUT I'M NOT SURE.  ANYONE ELSE?

NAH. EVERYONE JUST TAKES %HH AND TURNS IT INTO AN OCTET. THE
SPECIAL 
HANDLING TAKES PLACE FOR THE + SIGN ONLY, ON BOTH SIDES.
IN FACT, EVERY SCRIPT MUST BE PREPARED TO GET A %20 FROM THE
CLIENT, AS 
WELL.

ND
-- 
> RäTSELND, WAS EIN ANTHROPOSOPH MIT UNTERWERFUNG ZU TUN
HAT...
                    ^^^^^^^^^^^^
[...] DIESES WORT GIBT SO VIELE STELLEN FüR EINEN SPELLING
FLAME HER, UND
DU GöNNST EINEM KEINE EINZIGE.    -- JEAN CLAUDE UND DAVID
KASTRUP IN DTL