You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/09/26 21:29:43 UTC

1.3 now blocking on path escaping in httpd operational logging

kfogel@collab.net writes:
> So, put me down as in favor of the URI-encoding solution for now.
> 
> I don't think anyone really objected to it, besides me, so I'll just
> commit that.

The world turns out to be more complex than we thought.  I started to
make this change, and soon encountered the following code in
subversion/mod_dav_svn/deadprops.c:

   /* Tell the logging subsystem about the revprop change. */
   apr_table_set(db->resource->info->r->subprocess_env, "SVN-ACTION",
                 apr_psprintf(db->resource->pool,
                              "revprop-change r%" SVN_REVNUM_T_FMT
                              " '%s'", db->resource->info->root.rev,
                              propname));

Looks like we don't just use single quotes to delimit paths, we use
them to delimit property names as well!  And converting that to
URI-encoding would be odd.  It would "work", but would it really be
appropriate?

I think this problem is now too deep to allow us to branch for 1.3
today, sorry.  We're going to have to figure out exactly how we want
to escape paths (and propnames, and anything else that comes up)
before we can consider the mod_dav_svn operational logging feature to
be complete.  Technically, we don't have to quote paths in the same
way as propnames -- but we do need *some* way to quote propnames.
Whether we use it (instead of URI-encoding) for paths as well is an
open question.

I don't have any brilliant ideas for quoting methods, beyond the
backtick-quoting (note that's backtick, not backslash) proposal in my
earlier mail.

   http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=105914

Thoughts?

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-09-26 at 16:29 -0500, kfogel@collab.net wrote:
> Looks like we don't just use single quotes to delimit paths, we use
> them to delimit property names as well!  And converting that to
> URI-encoding would be odd.  It would "work", but would it really be
> appropriate?

I'm not sure what's so bad about URI-encoding things that aren't URIs.

Inventing a whole new escaping system seems like a bad idea, especially
since URI-encoding is perfectly readable for the 99% of pathnames and
property names which don't contain special characters.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Sep 27, 2005 at 10:11:22AM -0700, Garrett Rooney wrote:
> On 27 Sep 2005 10:51:00 -0500, kfogel@collab.net <kf...@collab.net> wrote:
> 
> > I hope I don't have to go much farther to convince you all how heinous
> > (1) and (2) would be :-).  Let's go with URI-escaping, because it's
> > the best we can do in a difficult situation.
> 
> That does seem to be the least insane path forward, so +1 on URI-escaping.

+1.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Ben Collins-Sussman <su...@red-bean.com>.
I just cookied this.

On 9/27/05, Branko Čibej <br...@xbc.nu> wrote:
> C. Michael Pilato wrote:
>
> >Then I must present my position reversal reversal petition, whereby I
> >petition to reverse the reversal of my position.  I'll follow with my
> >updated position proposition, in which I propose that after reversing
> >the reversal of my position, my position is now undefined.
> >
> >
> Only if the reversal space has helicoidal topology, and each reversal
> step ends with a rotation in the n+1-dimensional reversal hyperspace.
> Then a reversal of a reversal can send your position outside the
> reversal space proper, making it in effect undefined in the confines of
> that space.
>
> Otherwise, a reversal of a reversal will always leave you in the same
> reversal space, even if it doesn't necessarily (in non-Euclidean spaces)
> place you in the original position.
>
> >I need a physician.
> >
> >
> I suggest a topologist instead. Even surgeons only operate in a
> (locally) Euclidean space -- although I admit it's often a fairly yucky
> space that can turn your stomach, but that transformation is
> topologically neutral..
>
> -- Brane
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 27 Sep 2005 10:51:00 -0500, kfogel@collab.net <kf...@collab.net> wrote:

> I hope I don't have to go much farther to convince you all how heinous
> (1) and (2) would be :-).  Let's go with URI-escaping, because it's
> the best we can do in a difficult situation.

That does seem to be the least insane path forward, so +1 on URI-escaping.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz <ju...@erenkrantz.com> writes:

> On Tue, Sep 27, 2005 at 11:01:12AM -0400, C. Michael Pilato wrote:
> > Justin Erenkrantz <ju...@erenkrantz.com> writes:
> > 
> > > > XML-escaping permits whitespace, meaning whitespace could not be our
> > > > delimiter.  And mixing encodings is nasty.
> > > 
> > > No, it doesn't.  Those would still be quoted by virtue of the log format
> > > string that the admin would set.
> > 
> > I'm ... not able to parse that.  Sorry, Justin.
> 
> The log format that the admin would use would look like (from Ben's
> initial email):

[...]

> Hope this clarifies things a bit.  -- justin

It does, but not for the reasons you might assume.  I had forgotten
that the tokens which make up an SVN-ACTION item (the action itself,
paths and revisions and such, etc.) are not first-class Apache log
tokens like USERNAME, TIMESTAMP, and, in fact, SVN-ACTION itself.

There was a time when I was talking with Ben about using multiple
envvars, like:

   SVN-ACTION
   SVN-PATH1
   SVN-REV1
   SVN-PATH2
   SVN-REV2
   SVN-PROPNAME
   SVN-PROPVAL
   ...

So that admins could customize their logs, but I forgot that we
dropped that because the complexity was overbearing.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Sep 27, 2005 at 11:01:12AM -0400, C. Michael Pilato wrote:
> Justin Erenkrantz <ju...@erenkrantz.com> writes:
> 
> > > XML-escaping permits whitespace, meaning whitespace could not be our
> > > delimiter.  And mixing encodings is nasty.
> > 
> > No, it doesn't.  Those would still be quoted by virtue of the log format
> > string that the admin would set.
> 
> I'm ... not able to parse that.  Sorry, Justin.

The log format that the admin would use would look like (from Ben's
initial email):

CustomLog logs/svnlog "%t %u \"%{SVN-ACTION}e\"" env=SVN-ACTION

An instance would look like (again, from Ben's email):

[29/Aug/2005:18:09:48 -0500] powerguy "changed value of revision property 'svn:log'"

(%t == time stamp; %u == user)

The 'escaped' quotes in the admin's format string would be real quotes
in the log.  However, by virtue of httpd's escape mechanisms, no quotes
from the passed-in value (or any high-bit 8-bit chars, etc.) would make
it through to the logs un-escaped.

Therefore, a white space isn't required to parse the log format.
Whitespace could be helpful for parsing the SVN-ACTION parameter - but
it would be just as trivial to indicate that any 'action values' related
to property names are XML-encoded.

Hope this clarifies things a bit.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On September 27, 2005 11:27:37 AM -0500 kfogel@collab.net wrote:

> That's exactly what I was looking at, but I didn't read that case
> carefully enough.  You're right, thanks for the correction.

It's not you: that code stinks.  I've submitted a patch to dev@httpd to 
make it more straightforward.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by kf...@collab.net.
André Malo <nd...@perlig.de> writes:
> Only \xx is used. Well, c2x is utilized, but the logging escaper replaces 
> the % with an x and precedes the whole sequence with an \ (-> \xhh).
> See ap_escape_logitem in server/util.c.

That's exactly what I was looking at, but I didn't read that case
carefully enough.  You're right, thanks for the correction.

Nevertheless, I still think URI-escaping is preferable, for the
reasons given in my earlier mail.

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by André Malo <nd...@perlig.de>.
* kfogel@collab.net wrote:

> > > As I was trying to make clear in IRC yesterday, UTF-8 isn't an
> > > option. httpd (via ap_escape_logitem function) will convert the
> > > high-bit values via the c2x() function - which will print the hex
> > > value anyway (%xx).

Ahem. No.

> Okay.  In other words, httpd essentially cannot log events about local
> (server) paths in a perfectly parseable way.  Furthermore, the
> escaping that httpd enforces actually uses *two* different escape
> codes: \ for some things, and % for others.  But only one of the
> sequences escapes itself; the other does not, and cannot be
> distinguished from that escape sequence simply appearing in the
> original string.

Only \xx is used. Well, c2x is utilized, but the logging escaper replaces 
the % with an x and precedes the whole sequence with an \ (-> \xhh).
See ap_escape_logitem in server/util.c.

nd
-- 
"Solides und umfangreiches Buch"
                                          -- aus einer Rezension

<http://pub.perlig.de/books.html#apache2>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Branko Čibej <br...@xbc.nu>.
C. Michael Pilato wrote:

>Then I must present my position reversal reversal petition, whereby I
>petition to reverse the reversal of my position.  I'll follow with my
>updated position proposition, in which I propose that after reversing
>the reversal of my position, my position is now undefined.
>  
>
Only if the reversal space has helicoidal topology, and each reversal 
step ends with a rotation in the n+1-dimensional reversal hyperspace. 
Then a reversal of a reversal can send your position outside the 
reversal space proper, making it in effect undefined in the confines of 
that space.

Otherwise, a reversal of a reversal will always leave you in the same 
reversal space, even if it doesn't necessarily (in non-Euclidean spaces) 
place you in the original position.

>I need a physician.
>  
>
I suggest a topologist instead. Even surgeons only operate in a 
(locally) Euclidean space -- although I admit it's often a fairly yucky 
space that can turn your stomach, but that transformation is 
topologically neutral..

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by kf...@collab.net.
"C. Michael Pilato" <cm...@collab.net> writes:
> > Here are the rules for logging within httpd (see gen_test_char.c):
> > 
> >         /* For logging, escape all control characters,
> >          * double quotes (because they delimit the request in the log
> >          * file)
> >          * backslashes (because we use backslash for escaping)
> >          * and 8-bit chars with the high bit set
> >          */
> > 
> > As I was trying to make clear in IRC yesterday, UTF-8 isn't an option.
> > httpd (via ap_escape_logitem function) will convert the high-bit values
> > via the c2x() function - which will print the hex value anyway
> > (%xx).
> 
> Gotcha.  And, ick.  And, ugh.  And, yuck.
> 
> Then I must present my position reversal reversal petition, whereby I
> petition to reverse the reversal of my position.  I'll follow with my
> updated position proposition, in which I propose that after reversing
> the reversal of my position, my position is now undefined.
> 
> I need a physician.

Just FYI, Mike, I read that aloud to Karen and we laughed for a full
minute :-).

Justin Erenkrantz wrote:
> As Paul Querna and myself mentioned, if Subversion needs to write UTF-8
> logs, we can't rely upon httpd's logging mechanisms.

Okay.  In other words, httpd essentially cannot log events about local
(server) paths in a perfectly parseable way.  Furthermore, the
escaping that httpd enforces actually uses *two* different escape
codes: \ for some things, and % for others.  But only one of the
sequences escapes itself; the other does not, and cannot be
distinguished from that escape sequence simply appearing in the
original string.

I need a moment to let this all sink in.  I understand the security
concerns that required escaping control chars and high-bit chars, but
having two different escape sequences (and one of them unescapable!)
is a pity.  Oh well.  We've done similar things in Subversion, I
suppose, when we need to fall back from an recoding failure.

   -* moment passes *-

Okay, mental adjustment made.  Now, despite Miha Vitorovic's mail,
pointing out this Apache error message with a file path in it:

  [Tue May 24 14:34:23 2005] [error] [client 192.168.225.98] File does not 
  exist: C:/Program Files/Apache Group/Apache2/htdocs/favicon.ico

...I still think URI-escaping is the best solution we can do here.

Given the limitations of httpd logging, our only other choice would be
to come up with a non-intuitive custom solution.  I mean that
literally: "non-intuitive" is not simply a synonym of "custom",
rather, our solution would have to be *both* custom and non-intuitive,
because we can't ever get an odd number of backslashes in the output.
For example:

1) We could use double backslash (\\) itself as the escape sequence.
   That is, any sequence of *two* backslashes in the log would be
   treated as one actual backslash by parsers, just to get to the
   first level of reduction, except for double quote, which would have
   to be treated specially (see example below).  Highly unusual, hard
   for humans to read, and results in escape sequences of varying
   length.  Yuck.

2) Use % as the escape sequence.  But then you'd have to look at the
   next *two* characters, since we'd no choice but to represent
   backslash with %\\ and double-quote with %\", as httpd is going to
   force the \ on us in those cases.  Hex sequences are already %xx,
   which is also two characters, so that's okay.  But when escaping a
   single quote, we'd have to add in some extra char ourselves, and
   worse, it couldn't be backslash, because httpd would double that!
   So we'd have to do %-' or %'' or something unusual like that.  Yuck.

Let's call URI-escaping scheme (0), to compare it with (1) and (2)
above.  Here's how this path would look

   has'quote"doublequote\backslash space

in the three schemes:

   0) has%27quote%22doublequote%5Cbackslash%20space

   1) has\\'quote\\\"doublequote\\\\backslash space

   2) has%-'quote%\"doublequote%\\backslash space

Note that in (1) and (2), we'd first have to run the path through our
*own* escaper.  Then it would go through httpd's escaper before
hitting the log file.  For (1), our escaper would double all
backslashes, and put a backslash in front of double quotes.  For (2),
our escaper would put % before double quotes, backslashes, and would
put %- before single quotes.  Parsing these is left as an exercise for
the reader, especially the clinically insane reader with lots of time
on her hands and a research lab funded by an eccentric millionaire at
her disposal.

I hope I don't have to go much farther to convince you all how heinous
(1) and (2) would be :-).  Let's go with URI-escaping, because it's
the best we can do in a difficult situation.

When I'd finished writing the above, I polled mail, and saw this:

Greg Hudson writes:
> I'm not sure what's so bad about URI-encoding things that aren't URIs.
> 
> Inventing a whole new escaping system seems like a bad idea, especially
> since URI-encoding is perfectly readable for the 99% of pathnames and
> property names which don't contain special characters.

Yup.  What he said.

Thoughts?  Consensus?

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz <ju...@erenkrantz.com> writes:

> > XML-escaping permits whitespace, meaning whitespace could not be our
> > delimiter.  And mixing encodings is nasty.
> 
> No, it doesn't.  Those would still be quoted by virtue of the log format
> string that the admin would set.

I'm ... not able to parse that.  Sorry, Justin.

> > After sleeping on this, I'd like to reverse my position.  I'm fine
> > with double-quote-wrapped, single-space-delimited, backslash-escaped,
> > UTF-8 output:
> > 
> >    "propset" "svn:log" "This is my first \"real\" propset."
> >    "checkout" "/trunk/My Documents/Miscellany"
> >    ...
> 
> httpd forbids high-bit 8-bit characters.

Ah!  I missed this part of the discussion, obviously.

> Here are the rules for logging within httpd (see gen_test_char.c):
> 
>         /* For logging, escape all control characters,
>          * double quotes (because they delimit the request in the log
>          * file)
>          * backslashes (because we use backslash for escaping)
>          * and 8-bit chars with the high bit set
>          */
> 
> As I was trying to make clear in IRC yesterday, UTF-8 isn't an option.
> httpd (via ap_escape_logitem function) will convert the high-bit values
> via the c2x() function - which will print the hex value anyway
> (%xx).

Gotcha.  And, ick.  And, ugh.  And, yuck.

Then I must present my position reversal reversal petition, whereby I
petition to reverse the reversal of my position.  I'll follow with my
updated position proposition, in which I propose that after reversing
the reversal of my position, my position is now undefined.

I need a physician.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Sep 27, 2005 at 10:32:27AM -0400, C. Michael Pilato wrote:
> Justin Erenkrantz <ju...@erenkrantz.com> writes:
> 
> > --On September 26, 2005 4:29:43 PM -0500 kfogel@collab.net wrote:
> > 
> > > Looks like we don't just use single quotes to delimit paths, we use
> > > them to delimit property names as well!  And converting that to
> > > URI-encoding would be odd.  It would "work", but would it really be
> > > appropriate?
> > 
> > XML-escaping is what we have to do on the wire for propnames.  I think
> > it makes sense to do that here too.  -- justin
> 
> XML-escaping permits whitespace, meaning whitespace could not be our
> delimiter.  And mixing encodings is nasty.

No, it doesn't.  Those would still be quoted by virtue of the log format
string that the admin would set.

> After sleeping on this, I'd like to reverse my position.  I'm fine
> with double-quote-wrapped, single-space-delimited, backslash-escaped,
> UTF-8 output:
> 
>    "propset" "svn:log" "This is my first \"real\" propset."
>    "checkout" "/trunk/My Documents/Miscellany"
>    ...

httpd forbids high-bit 8-bit characters.

Here are the rules for logging within httpd (see gen_test_char.c):

        /* For logging, escape all control characters,
         * double quotes (because they delimit the request in the log
         * file)
         * backslashes (because we use backslash for escaping)
         * and 8-bit chars with the high bit set
         */

As I was trying to make clear in IRC yesterday, UTF-8 isn't an option.
httpd (via ap_escape_logitem function) will convert the high-bit values
via the c2x() function - which will print the hex value anyway (%xx).

httpd's goal is to not allow unsafe characters in the logs that could
corrupt the admin's screen.  We had too many reports of security risks
by allowing such characters in the logs.

As Paul Querna and myself mentioned, if Subversion needs to write UTF-8
logs, we can't rely upon httpd's logging mechanisms.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz <ju...@erenkrantz.com> writes:

> --On September 26, 2005 4:29:43 PM -0500 kfogel@collab.net wrote:
> 
> > Looks like we don't just use single quotes to delimit paths, we use
> > them to delimit property names as well!  And converting that to
> > URI-encoding would be odd.  It would "work", but would it really be
> > appropriate?
> 
> XML-escaping is what we have to do on the wire for propnames.  I think
> it makes sense to do that here too.  -- justin

XML-escaping permits whitespace, meaning whitespace could not be our
delimiter.  And mixing encodings is nasty.

After sleeping on this, I'd like to reverse my position.  I'm fine
with double-quote-wrapped, single-space-delimited, backslash-escaped,
UTF-8 output:

   "propset" "svn:log" "This is my first \"real\" propset."
   "checkout" "/trunk/My Documents/Miscellany"
   ...

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by kf...@collab.net.
Justin Erenkrantz <ju...@erenkrantz.com> writes:
> --On September 26, 2005 4:29:43 PM -0500 kfogel@collab.net wrote:
> > Looks like we don't just use single quotes to delimit paths, we use
> > them to delimit property names as well!  And converting that to
> > URI-encoding would be odd.  It would "work", but would it really be
> > appropriate?
> 
> XML-escaping is what we have to do on the wire for propnames.  I think
> it makes sense to do that here too.  -- justin

Just to make sure: are you proposing that we escape propnames
differently from paths, in the operational logs?

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.3 now blocking on path escaping in httpd operational logging

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On September 26, 2005 4:29:43 PM -0500 kfogel@collab.net wrote:

> Looks like we don't just use single quotes to delimit paths, we use
> them to delimit property names as well!  And converting that to
> URI-encoding would be odd.  It would "work", but would it really be
> appropriate?

XML-escaping is what we have to do on the wire for propnames.  I think it 
makes sense to do that here too.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org