On Mon, 22 Oct 2007 21:03:25 +0200 Günther Gsenger <[EMAIL PROTECTED]> 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.org/
