On Mon, 2 Jun 2003 23:23, Henrik Nordstrom wrote: > Looking reasonable, but data inserted into URLs need to be URL escaped > with rfc1738_escape_part() (not rfc1738_escape) before it is inserted > into the URL or else there will be issues in most uses.. while data > inserted into error pages need to be escaped by rfc1738_escape().
Thanks Henrik. I've attached a slightly modified patch that escapes only the substituted data with rfc1738_escape_part(). I understand now why rfc1738_escape_part() is needed instead of rfc1738_escape(), but as for escaping data before it is inserted in the URL- is this so squid can assume that the deny_info url (the part without the format strings) is already escaped? Eg: deny_info http://myserver/test.cgi?[E=%E] myacl Original patch: http://myserver/test.cgi?%5bE=%5bNo%20Error%5d%5d New patch: http://myserver/test.cgi?[E=%5bNo%20Error%5d] Also, data in error pages is already escaped with html_quote() if required. I haven't changed that behaviour. Regards Gerard
diff -uNr squid-3.0.DEVEL-20030601/src/cf.data.pre squid-3.0.DEVEL-20030601.new/src/cf.data.pre --- squid-3.0.DEVEL-20030601/src/cf.data.pre 2003-05-30 01:54:07.000000000 +1000 +++ squid-3.0.DEVEL-20030601.new/src/cf.data.pre 2003-06-02 22:16:38.000000000 +1000 @@ -2881,8 +2881,13 @@ and put them into the configured errors/ directory. Alternatively you can specify an error URL. The browsers will then - get redirected (302) to the specified URL. %s in the redirection - URL will be replaced by the requested URL. + get redirected (302) to the specified URL. Placeholders such as %U + can be used in the error URL to include extra information. + + See the Squid FAQ section on Customizable Error Messages + (http://www.squid-cache.org/Doc/FAQ/FAQ-19.html#custom-err-msgs) + for more information on placeholders that can be used in ERR_ pages + and error URLs. Alternatively you can tell Squid to reset the TCP connection by specifying TCP_RESET. diff -uNr squid-3.0.DEVEL-20030601/src/errorpage.cc squid-3.0.DEVEL-20030601.new/src/errorpage.cc --- squid-3.0.DEVEL-20030601/src/errorpage.cc 2003-03-10 14:56:38.000000000 +1000 +++ squid-3.0.DEVEL-20030601.new/src/errorpage.cc 2003-06-03 23:07:25.000000000 +1000 @@ -101,9 +101,9 @@ static const char *errorFindHardText(err_type type); static ErrorDynamicPageInfo *errorDynamicPageInfoCreate(int id, const char *page_name); static void errorDynamicPageInfoDestroy(ErrorDynamicPageInfo * info); -static MemBuf errorBuildContent(ErrorState * err); +static MemBuf errorBuildContent(ErrorState * err, int buildUrl = 0); static int errorDump(ErrorState * err, MemBuf * mb); -static const char *errorConvert(char token, ErrorState * err); +static const char *errorConvert(char token, ErrorState * err, int do_quote = 1); static CWCB errorSendComplete; @@ -590,12 +590,11 @@ */ static const char * -errorConvert(char token, ErrorState * err) +errorConvert(char token, ErrorState * err, int do_quote) { request_t *r = err->request; static MemBuf mb = MemBufNULL; const char *p = NULL; /* takes priority over mb if set */ - int do_quote = 1; memBufReset(&mb); @@ -826,10 +825,13 @@ if (strchr(name, ':')) { /* Redirection */ - char *quoted_url = rfc1738_escape_part(errorConvert('u', err)); + MemBuf redirectUrl; + /* replace format strings in the URL with rfc1738 escaped data */ + redirectUrl = errorBuildContent(err, true); httpReplySetHeaders(rep, version, HTTP_MOVED_TEMPORARILY, NULL, "text/html", 0, 0, squid_curtime); - httpHeaderPutStrf(&rep->header, HDR_LOCATION, name, quoted_url); + httpHeaderPutStr(&rep->header, HDR_LOCATION, redirectUrl.buf); httpHeaderPutStrf(&rep->header, HDR_X_SQUID_ERROR, "%d %s\n", err->httpStatus, "Access Denied"); + memBufClean(&redirectUrl); } else { MemBuf content = errorBuildContent(err); httpReplySetHeaders(rep, version, err->httpStatus, NULL, "text/html", content.size, 0, squid_curtime); @@ -851,7 +853,7 @@ } static MemBuf -errorBuildContent(ErrorState * err) +errorBuildContent(ErrorState * err, int buildUrl) { MemBuf content; const char *m; @@ -860,14 +862,14 @@ assert(err != NULL); assert(err->page_id > ERR_NONE && err->page_id < error_page_count); memBufDefInit(&content); - m = error_text[err->page_id]; + m = buildUrl ? errorPageName(err->page_id) : error_text[err->page_id]; assert(m); while ((p = strchr(m, '%'))) { - memBufAppend(&content, m, p - m); /* copy */ - t = errorConvert(*++p, err); /* convert */ - memBufPrintf(&content, "%s", t); /* copy */ - m = p + 1; /* advance */ + memBufAppend(&content, m, p - m); /* copy */ + t = errorConvert(*++p, err, buildUrl ? 0 : 1); /* convert */ + memBufPrintf(&content, "%s", buildUrl ? rfc1738_escape_part(t) : t); + m = p + 1; /* advance */ } if (*m)
