Re: [PATCH] Request URI logging for malformed requests

 On Tue, 01 Mar 2011 20:29:51 +0200, Tsantilas Christos wrote:
> This patch is forgotten too.
> Any comments?
>

 Just doing a quick audit now...

 src/cf.data.pre
  - remove comment change on "encode" and "chop options. They are
 redundant after the earlier comment that this directive also applies to
 logging.
  - on "deny" option should be "The whitespaces are removed before
 logging."
  - on the "allow" option should be "WARNING: The URI is logged with
 whitespaces."
   - also the sentence about redirectors also needs a WARNING where it
 has "Note"
   - both of these warnings should start on their own line inside the
 option description.
 ie
 "
             allow: The request is allowed and the URI is not changed. The
                  whitespace characters remain in the URI.
                 WARNING: The whitespace is passed to redirector
 processes
                 if they are in use.
                 WARNING: The URI is logged with whitespaces.
 "

 src/client_side.cc:
  - the logic is wrong
  - in setLogUri() you do not need to call stringHasCntl(uri). The
 unescape call will do a one-pass scan and check these as it goes.
    also an old bug is now visible, stringHasCntl() does not check the
 full set of characters that need encoding :(

  With that fixed its clear that stringHasWhitespace() needs to be
 performed *before* any escaping and duplicating is done. Some shuffling
 can avoid doing that scan pass entirely on most config setups.

 What I think is needed is:

 if (!cleanUrl)
 ...
 else {
     switch (Config.uri_whitespace) {
     case URI_WHITESPACE_ALLOW:
         http->log_uri = xstrndup(uri, MAX_URL);
         break;

     case URI_WHITESPACE_ENCODE:
         http->log_uri = xstrndup(rfc1738_escape_unescaped(uri),
 MAX_URL);
         break;

     case URI_WHITESPACE_CHOP:
         http->log_uri = xstrndup(uri,
 min(MAX_URL,strcspn(http->log_uri, w_space)));
         break;

     case URI_WHITESPACE_DENY:
     case URI_WHITESPACE_STRIP:
     default:
     {
          char *q = static_cast<char*>(xmalloc(strlen(uri)*3 + 1));
          const char *t = uri;
          while (*t) {
              if (!xisspace(*t))
                  *q++ = *t;
              t++;
         }
         *q = '\0';
         http->log_uri = xstrndup(rfc1738_escape_unescaped(q), MAX_URL);
         xfree(q);
     }
 }

 Amos
Received on Wed Mar 02 2011 - 01:01:00 MST