From: Amos Jeffries Date: Sat, 10 Nov 2012 06:40:21 +0000 (+1300) Subject: Fix URL-decode logics consistency X-Git-Tag: SQUID_3_4_0_1~471^2~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=05e5285440fcf3537422a5282ebd0d35898ef9fd;p=thirdparty%2Fsquid.git Fix URL-decode logics consistency * always perform URL-decode on only token form of values (\-escaping always removed on all values prior to URL-decode) * document change in URL-encoding for external_acl_type config * deprecate external_acl type quote= and protocol= options * remove unneeded 'urlQuoting' parameter from HelperReply parser --- diff --git a/src/HelperReply.cc b/src/HelperReply.cc index de2f29d134..26f0de842a 100644 --- a/src/HelperReply.cc +++ b/src/HelperReply.cc @@ -9,15 +9,15 @@ #include "rfc1738.h" #include "SquidString.h" -HelperReply::HelperReply(char *buf, size_t len, bool urlQuoting) : +HelperReply::HelperReply(char *buf, size_t len) : result(HelperReply::Unknown), whichServer(NULL) { - parse(buf,len,urlQuoting); + parse(buf,len); } void -HelperReply::parse(char *buf, size_t len, bool urlQuoting) +HelperReply::parse(char *buf, size_t len) { // check we have something to parse if (!buf || len < 1) { @@ -105,7 +105,7 @@ HelperReply::parse(char *buf, size_t len, bool urlQuoting) // NULL-terminate so the helper callback handlers do not buffer-overrun other_.terminate(); - parseResponseKeys(urlQuoting); + parseResponseKeys(); // Hack for backward-compatibility: BH used to be a text message... if (other().hasContent() && result == HelperReply::BrokenHelper) { @@ -115,7 +115,7 @@ HelperReply::parse(char *buf, size_t len, bool urlQuoting) } void -HelperReply::parseResponseKeys(bool urlQuotingValues) +HelperReply::parseResponseKeys() { // parse a "key=value" pair off the 'other()' buffer. while(other().hasContent()) { @@ -130,9 +130,9 @@ HelperReply::parseResponseKeys(bool urlQuotingValues) String key(other().content()); // the value may be a quoted string or a token - // XXX: eww. update strwordtok() to be zero-copy + const bool urlDecode = (*p != '"'); // check before moving p. char *v = strwordtok(NULL, &p); - if (v != NULL && (p-v) > 2) // 1-octet %-escaped requires 3 bytes + if (v != NULL && urlDecode && (p-v) > 2) // 1-octet %-escaped requires 3 bytes rfc1738_unescape(v); String value = v; diff --git a/src/HelperReply.h b/src/HelperReply.h index 5f06136f85..867cff833c 100644 --- a/src/HelperReply.h +++ b/src/HelperReply.h @@ -31,7 +31,7 @@ public: // create/parse details from the msg buffer provided // XXX: buf should be const but parse() needs non-const for now - HelperReply(char *buf, size_t len, bool urlQuoting = false); + HelperReply(char *buf, size_t len); const MemBuf &other() const { return other_; } @@ -45,10 +45,11 @@ public: * line := [ result ] *#( key-pair ) * key-pair := OWS token '=' ( quoted-string | token ) * - * \param urlQuoting decode note values using RFC 1738 decoder. (default: use quoted-string instead) + * token are URL-decoded. + * quoted-string are \-escape decoded and the quotes are stripped. */ // XXX: buf should be const but we may need strwordtok() and rfc1738_unescape() - void parse(char *buf, size_t len, bool urlQuoting = false); + void parse(char *buf, size_t len); public: /// The helper response 'result' field. @@ -70,7 +71,7 @@ public: CbcPointer whichServer; private: - void parseResponseKeys(bool urlQuotingValues); + void parseResponseKeys(); /// the remainder of the line MemBuf other_; diff --git a/src/cf.data.pre b/src/cf.data.pre index fc19687346..d21df75098 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -630,25 +630,33 @@ DOC_START General result syntax: - OK/ERR keyword=value ... + [channel-ID] OK/ERR/BH keyword=value ... Defined keywords: user= The users name (login) password= The users password (for login= cache_peer option) - message= Message describing the reason. Available as %o - in error pages - tag= Apply a tag to a request (for both ERR and OK results) - Only sets a tag, does not alter existing tags. + message= Message describing the reason for this response. + Available as %o in error pages. Useful on (ERR and BH results). + tag= Apply a tag to a request. Only sets a tag once, + does not alter existing tags. log= String to be logged in access.log. Available as %ea in logformat specifications - If protocol=3.0 (the default) then URL escaping is used to protect - each value in both requests and responses. + Any keywords may be sent on any response whether OK, ERR or BH. - If using protocol=2.5 then all values need to be enclosed in quotes - if they may contain whitespace, or the whitespace escaped using \. - And quotes or \ characters within the keyword value must be \ escaped. + All response keyword values need to be a single token with URL + escaping, or enclosed in double quotes (") and escaped using \ on + any quotes, whitespace or \ characters within the value + For example: user="John\ Smith" or "J.\ O\'Brien" + + Request values sent to the helper are URL escaped to protect + each value in requests against whitespaces. + + If using protocol=2.5 then the request sent to the helper is not + URL escaped to protect against whitespace. + + NOTE: protocol=3.0 is deprecated as no longer necessary. When using the concurrency= option the protocol is changed by introducing a query channel tag infront of the request/response. diff --git a/src/external_acl.cc b/src/external_acl.cc index de15fe488d..952e95c02e 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -361,10 +361,16 @@ parse_externalAclHelper(external_acl ** list) } else if (strcmp(token, "protocol=2.5") == 0) { a->quote = external_acl::QUOTE_METHOD_SHELL; } else if (strcmp(token, "protocol=3.0") == 0) { + debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option protocol=3.0 is deprecated. Remove this from your config."); + a->quote = external_acl::QUOTE_METHOD_URL; + } else if (strcmp(token, "protocol=3.4") == 0) { + debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option protocol=3.4 is the default. Remove this from your config."); a->quote = external_acl::QUOTE_METHOD_URL; } else if (strcmp(token, "quote=url") == 0) { + debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option quote=url is deprecated. Remove this from your config."); a->quote = external_acl::QUOTE_METHOD_URL; } else if (strcmp(token, "quote=shell") == 0) { + debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option quote=shell is deprecated. Use protocol=2.5 if still needed."); a->quote = external_acl::QUOTE_METHOD_SHELL; /* INET6: allow admin to configure some helpers explicitly to @@ -549,6 +555,9 @@ dump_externalAclHelper(StoreEntry * sentry, const char *name, const external_acl if (node->cache) storeAppendPrintf(sentry, " cache=%d", node->cache_size); + if (node->quote == external_acl::QUOTE_METHOD_SHELL) + storeAppendPrintf(sentry, " protocol=2.5"); + for (format = node->format; format; format = format->next) { switch (format->type) { @@ -1302,9 +1311,8 @@ free_externalAclState(void *data) * * Other keywords may be added to the protocol later * - * value needs to be enclosed in quotes if it may contain whitespace, or - * the whitespace escaped using \ (\ escaping obviously also applies to - * any " characters) + * value needs to be URL-encoded or enclosed in double quotes (") + * with \-escaping on any whitespace, quotes, or slashes (\). */ static void externalAclHandleReply(void *data, const HelperReply &reply)