On 28/08/2008, at 9:26 PM, Amos Jeffries wrote:
Benno Rice wrote:
[snip]
------------------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: [EMAIL PROTECTED]
# target_branch: file:///home/benno/squid-work/squid3-repo/trunk/
# testament_sha1: e5013df20b582dbb9a4c9124644e6c521edd48ea
# timestamp: 2008-08-28 15:14:45 +1000
# base_revision_id: [EMAIL PROTECTED]
# 58a98r50hgyj5e27
# # Begin patch
=== modified file 'src/HttpRequestMethod.cc'
--- src/HttpRequestMethod.cc 2008-06-20 04:43:01 +0000
+++ src/HttpRequestMethod.cc 2008-08-28 05:12:17 +0000
@@ -246,7 +246,7 @@
/* all others */
case METHOD_OTHER:
default:
- return false; // be conservative: we do not know some
methods specs
+ return true; // RFC says to purge if we don't know the
method
Fair enough. But which RFC and section/subsection (maybe paragraph)?
There is a unit-test needed for this function too.
Ref: src/tests/testHttpRequestMethod.*
RFC 2616, 13.10, last paragraph. I've added a comment to this effect.
How do I run the unit test suite?
}
return false; // not reached
=== modified file 'src/Server.cc'
--- src/Server.cc 2008-07-22 12:33:41 +0000
+++ src/Server.cc 2008-08-28 05:12:17 +0000
@@ -402,11 +402,20 @@
// purges entries that match the value of a given HTTP [response]
header
static void
-purgeEntriesByHeader(const char *reqUrl, HttpMsg *rep,
http_hdr_type hdr)
+purgeEntriesByHeader(const HttpRequest *req, const char *reqUrl,
HttpMsg *rep, http_hdr_type hdr)
{
- if (const char *url = rep->header.getStr(hdr))
- if (sameUrlHosts(reqUrl, url)) // prevent purging DoS, per
RFC 2616
+ if (const char *url = rep->header.getStr(hdr)) {
+ const char *absUrl = urlAbsolute(req, url);
+ if (absUrl != NULL) {
+ url = absUrl;
+ }
+ if (sameUrlHosts(reqUrl, url)) { // prevent purging DoS,
per RFC 2616
purgeEntriesByUrl(url);
+ }
+ if (absUrl != NULL) {
+ xfree((void *)absUrl);
+ }
safe_free(absUrl);
Done.
+ }
}
// some HTTP methods should purge matching cache entries
@@ -425,8 +434,8 @@
const char *reqUrl = urlCanonical(request);
debugs(88, 5, "maybe purging due to " <<
RequestMethodStr(request->method) << ' ' << reqUrl);
purgeEntriesByUrl(reqUrl);
- purgeEntriesByHeader(reqUrl, theFinalReply, HDR_LOCATION);
- purgeEntriesByHeader(reqUrl, theFinalReply,
HDR_CONTENT_LOCATION);
+ purgeEntriesByHeader(request, reqUrl, theFinalReply,
HDR_LOCATION);
+ purgeEntriesByHeader(request, reqUrl, theFinalReply,
HDR_CONTENT_LOCATION);
}
// called (usually by kids) when we have final (possibly adapted)
reply headers
=== modified file 'src/protos.h'
--- src/protos.h 2008-07-17 12:38:06 +0000
+++ src/protos.h 2008-08-28 05:12:17 +0000
@@ -638,6 +638,7 @@
SQUIDCEXTERN void urlInitialize(void);
SQUIDCEXTERN HttpRequest *urlParse(const HttpRequestMethod&, char
*, HttpRequest *request = NULL);
SQUIDCEXTERN const char *urlCanonical(HttpRequest *);
+SQUIDCEXTERN const char *urlAbsolute(const HttpRequest *, const
char *);
SQUIDCEXTERN char *urlRInternal(const char *host, u_short port,
const char *dir, const char *name);
SQUIDCEXTERN char *urlInternal(const char *dir, const char *name);
SQUIDCEXTERN int matchDomainName(const char *host, const char
*domain);
Most of the above should be in src/URL.h.
Can you at least add the new ones there. The 'CEXTERN declaration is
fine until the whole URL area gets cleaned up.
I think I'd rather leave it where it is until they all get moved,
otherwise it could cause confusion when it can't be found.
=== modified file 'src/url.cc'
--- src/url.cc 2008-04-10 11:29:39 +0000
+++ src/url.cc 2008-08-28 05:12:17 +0000
@@ -532,6 +532,70 @@
return buf;
}
Now we get the bits I have not learned much of, so lessons may be in
order.
+const char *
+urlAbsolute(const HttpRequest * req, const char *relUrl)
+{
+ LOCAL_ARRAY(char, portbuf, 32);
+ LOCAL_ARRAY(char, urlbuf, MAX_URL);
LOCAL_ARRAY is unfortunately not thread safe. Someone else may
argue, but I prefer to see them dead.
Replaced with stack-defined arrays.
+ char *path, *last_slash;
+
+ if (relUrl == NULL) {
+ return (NULL);
+ }
+ if (req->method.id() == METHOD_CONNECT) {
+ return (NULL);
+ }
+ if (strchr(relUrl, ':') != NULL) {
+ return (NULL);
Hmm, can relURL not contain the dynamic-part?
I'd expect those to have ':' sometimes.
Hrm. I'll see if I can refine this. Any suggestions?
[snip]
+ } else {
+ portbuf[0] = '\0';
+ if (req->port != urlDefaultPort(req->protocol)) {
+ snprintf(portbuf, 32, ":%d", req->port);
+ }
+ if (relUrl[0] == '/') {
+ snprintf(urlbuf, MAX_URL, "%s://%s%s%s%s%s",
+ ProtocolStr[req->protocol],
+ req->login,
+ *req->login ? "@" : null_string,
+ req->GetHost(),
+ portbuf,
+ relUrl
+ );
+ } else {
+ path = xstrdup(req->urlpath.buf());
Should be no need for that copy. Just construct the urlBuf
incrementally and some ptr math when it comes to the last_slash
memcpy().
That would also remove the need for two different snprintf patterns
below.
Incrementally? Do you mean as in using <<? Can you give me an
example? (C++ is not my strong suit =))
--
Benno Rice
[EMAIL PROTECTED]
