[Success!] [MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-02-08 Thread Bundle Buggy

This change has been merged.
Previous status: Approved

For details, see: 
http://bundlebuggy.aaronbentley.com/project/squid/request/%3C49759168.2080600%40treenet.co.nz%3E

Project: Squid


Re: [MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-02-06 Thread Amos Jeffries

Amos Jeffries has voted approve.
Status is now: Approved


For details, see: 
http://bundlebuggy.aaronbentley.com/project/squid/request/%3C49759168.2080600%40treenet.co.nz%3E

Project: Squid


Re: [MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-02-06 Thread Henrik Nordstrom

hnordstrom has voted approve.
Status is now: Semi-approved


For details, see: 
http://bundlebuggy.aaronbentley.com/project/squid/request/%3C49759168.2080600%40treenet.co.nz%3E

Project: Squid


[MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-01-20 Thread Amos Jeffries
This attempt builds on Henriks re-work of the client-request to 
server-request cloning done since the last attempt was made at closing 
this bug.


Adds all RFC 2616 listed Hop-by-hop headers to the clone selection test 
as 'ignore' cases unless otherwise handled already.


The test for whether they exist in Connection: is moved to the default 
case as an inline. Which reduces the code a fair bit and prevents the 
side case where a specially handled header gets ignored because the 
client explicitly added it to Connection: when it did not have to.



This method sets up a background default of not passing the hop-by-hop 
headers while allowing any code which explicitly sets or copies the 
headers across to operate as before without interference.
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squ...@treenet.co.nz-20090120085104-4jxpmbmnuowss2za
# target_branch: file:///src/squid/bzr/trunk/
# testament_sha1: 206a5d4bc37533ae3fef2dd39d4516202227a931
# timestamp: 2009-01-20 21:52:26 +1300
# base_revision_id: squ...@treenet.co.nz-20090118082924-\
#   c72yl5o47sx7e7fd
# 
# Begin patch
=== modified file 'src/HttpHeaderTools.cc'
--- src/HttpHeaderTools.cc  2008-12-24 13:35:21 +
+++ src/HttpHeaderTools.cc  2009-01-20 08:51:04 +
@@ -182,7 +182,7 @@
 return res;
 }
 
-/* returns true iff "m" is a member of the list */
+/** returns true iff "m" is a member of the list */
 int
 strListIsMember(const String * list, const char *m, char del)
 {

=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc  2009-01-08 13:45:29 +
+++ src/HttpRequest.cc  2009-01-20 08:51:04 +
@@ -326,6 +326,7 @@
header.len + 2;
 }
 
+#if DEAD_CODE // 2009-01-20: inlined this with its ONLY caller 
(copyOneHeader...)
 /**
  * Returns true if HTTP allows us to pass this header on.  Does not
  * check anonymizer (aka header_access) configuration.
@@ -341,6 +342,7 @@
 
 return 1;
 }
+#endif
 
 /* sync this routine when you update HttpRequest struct */
 void

=== modified file 'src/http.cc'
--- src/http.cc 2009-01-13 05:28:23 +
+++ src/http.cc 2009-01-20 08:51:04 +
@@ -72,8 +72,8 @@
 static const char *const crlf = "\r\n";
 
 static void httpMaybeRemovePublic(StoreEntry *, http_status);
-static void copyOneHeaderFromClientsideRequestToUpstreamRequest(const 
HttpHeaderEntry *e, String strConnection, HttpRequest * request, HttpRequest * 
orig_request,
-HttpHeader * hdr_out, int we_do_ranges, http_state_flags);
+static void copyOneHeaderFromClientsideRequestToUpstreamRequest(const 
HttpHeaderEntry *e, const String strConnection, HttpRequest * request, const 
HttpRequest * orig_request,
+HttpHeader * hdr_out, const int we_do_ranges, const http_state_flags);
 
 HttpStateData::HttpStateData(FwdState *theFwdState) : 
AsyncJob("HttpStateData"), ServerStateData(theFwdState),
 lastChunk(0), header_bytes_read(0), reply_bytes_read(0), 
httpChunkDecoder(NULL)
@@ -1647,20 +1647,22 @@
 strConnection.clean();
 }
 
+/**
+ * Decides whether a particular header may be cloned from the received Clients 
request
+ * to our outgoing fetch request.
+ */
 void
-copyOneHeaderFromClientsideRequestToUpstreamRequest(const HttpHeaderEntry *e, 
String strConnection, HttpRequest * request, HttpRequest * orig_request, 
HttpHeader * hdr_out, int we_do_ranges, http_state_flags flags)
+copyOneHeaderFromClientsideRequestToUpstreamRequest(const HttpHeaderEntry *e, 
const String strConnection, HttpRequest * request, const HttpRequest * 
orig_request, HttpHeader * hdr_out, const int we_do_ranges, const 
http_state_flags flags)
 {
 debugs(11, 5, "httpBuildRequestHeader: " << e->name.buf() << ": " << 
e->value.buf());
 
-if (!httpRequestHdrAllowed(e, &strConnection)) {
-debugs(11, 2, "'" << e->name.buf() << "' header denied by 
anonymize_headers configuration");
-return;
-}
-
 switch (e->id) {
 
+/** \title RFC 2616 sect 13.5.1 - Hop-by-Hop headers which Squid should not 
pass on. */
+
 case HDR_PROXY_AUTHORIZATION:
-/* Only pass on proxy authentication to peers for which
+/** \par Proxy-Authorization:
+ * Only pass on proxy authentication to peers for which
  * authentication forwarding is explicitly enabled
  */
 
@@ -1672,16 +1674,31 @@
 
 break;
 
+/** \title RFC 2616 sect 13.5.1 - Hop-by-Hop headers which Squid does not pass 
on. */
+
+case HDR_CONNECTION:  /** \par Connection: */
+case HDR_TE:  /** \par TE: */
+case HDR_KEEP_ALIVE:  /** \par Keep-Alive: */
+case HDR_PROXY_AUTHENTICATE:  /** \par Proxy-Authenticate: */
+case HDR_TRAILERS:/** \par Trailers: */
+case HDR_UPGRADE: /** \par Upgrade: */
+case HDR_TRANSFER_ENCODING:   /** \par Transfer-Encoding: */
+break;
+
+
+/** \title OTHER headers I haven't bothered to track down yet. */
+
 case HDR_AUTHORIZATION:
-/* Pass on WWW authentication */
+ 

Re: [MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-01-20 Thread Amos Jeffries

Fubar. Real bundle coming soon.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE5 or 3.0.STABLE11
  Current Beta Squid 3.1.0.3


[MERGE] Bug 419: Hop by Hop headers MUST NOT be forwarded (attempt 2)

2009-01-20 Thread Amos Jeffries
This attempt builds on Henriks re-work of the client-request to 
server-request cloning done since the last attempt was made at closing 
this bug.


Adds all RFC 2616 listed Hop-by-hop headers to the clone selection test 
as 'ignore' cases unless otherwise handled already.


The test for whether they exist in Connection: is moved to the default 
case as an inline. Which reduces the code a fair bit and prevents the 
side case where a specially handled header gets ignored because the 
client explicitly added it to Connection: when it did not have to.



This method sets up a background default of not passing the hop-y-op 
headers while allowing any code which explicitly sets or copies the 
headers across to operate as before without interference.
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squ...@treenet.co.nz-20090118082924-c72yl5o47sx7e7fd
# target_branch: file:///src/squid/bzr/trunk/
# testament_sha1: 8e017a5e58aab84043111ffbf80e8e0c587cf697
# timestamp: 2009-01-20 20:59:39 +1300
# base_revision_id: squ...@treenet.co.nz-20090118082924-\
#   c72yl5o47sx7e7fd
# 
# Begin patch
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWYUFw0YAABHfgAAQQGFzUBIA
AACv794QIABkRR6nqaep6gZPUA9IxCjJqaBo09JpgjIMQ3m0vN/kfmJsLk0MHQ1W4qiRcX7dBKNJ
lorThQidj3YxTykvD4J5hd1FQti/6hzQ9gCghgPfGQ8nyFtaDiAA5JCZ68iW34u5IpwoSEKC4aMA