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

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Tue, 20 Jan 2009 21:55:04 +1300

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: squid3_at_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: squid3_at_treenet.co.nz-20090118082924-\
# c72yl5o47sx7e7fd
#
# Begin patch
=== modified file 'src/HttpHeaderTools.cc'
--- src/HttpHeaderTools.cc 2008-12-24 13:35:21 +0000
+++ src/HttpHeaderTools.cc 2009-01-20 08:51:04 +0000
@@ -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 +0000
+++ src/HttpRequest.cc 2009-01-20 08:51:04 +0000
@@ -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 +0000
+++ src/http.cc 2009-01-20 08:51:04 +0000
@@ -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 */
+ /** \par WWW-Authorization:
+ * Pass on WWW authentication */
 
         if (!flags.originpeer) {
             hdr_out->addEntry(e->clone());
         } else {
- /* In accelerators, only forward authentication if enabled
+ /** \note In accelerators, only forward authentication if enabled
+ * by login=PASS or login=PROXYPASS
              * (see also below for proxy->server authentication)
              */
-
             if (orig_request->peer_login &&
                     (strcmp(orig_request->peer_login, "PASS") == 0 ||
                      strcmp(orig_request->peer_login, "PROXYPASS") == 0)) {
@@ -1692,7 +1709,7 @@
         break;
 
     case HDR_HOST:
- /*
+ /** \par Host:
          * Normally Squid rewrites the Host: header.
          * However, there is one case when we don't: If the URL
          * went through our redirector and the admin configured
@@ -1717,8 +1734,9 @@
         break;
 
     case HDR_IF_MODIFIED_SINCE:
- /* append unless we added our own;
- * note: at most one client's ims header can pass through */
+ /** \par If-Modified-Since:
+ * append unless we added our own;
+ * \note at most one client's ims header can pass through */
 
         if (!hdr_out->has(HDR_IF_MODIFIED_SINCE))
             hdr_out->addEntry(e->clone());
@@ -1726,6 +1744,8 @@
         break;
 
     case HDR_MAX_FORWARDS:
+ /** \par Max-Forwards:
+ * pass only on TRACE requests */
         if (orig_request->method == METHOD_TRACE) {
             const int hops = e->getInt();
 
@@ -1736,7 +1756,9 @@
         break;
 
     case HDR_VIA:
- /* If Via is disabled then forward any received header as-is */
+ /** \par Via:
+ * If Via is disabled then forward any received header as-is.
+ * Otherwise leave for explicit updated addition later. */
 
         if (!Config.onoff.via)
             hdr_out->addEntry(e->clone());
@@ -1748,6 +1770,8 @@
     case HDR_IF_RANGE:
 
     case HDR_REQUEST_RANGE:
+ /** \par Range:, If-Range:, Request-Range:
+ * Only pass if we accept ranges */
         if (!we_do_ranges)
             hdr_out->addEntry(e->clone());
 
@@ -1755,22 +1779,32 @@
 
     case HDR_PROXY_CONNECTION:
 
- case HDR_CONNECTION:
-
     case HDR_X_FORWARDED_FOR:
 
     case HDR_CACHE_CONTROL:
- /* append these after the loop if needed */
+ /** \par Proxy-Connaction:, X-Forwarded-For:, Cache-Control:
+ * handled specially by Squid, so leave off for now.
+ * append these after the loop if needed */
         break;
 
     case HDR_FRONT_END_HTTPS:
+ /** \par Front-End-Https:
+ * Pass thru only if peer is configured with front-end-https */
         if (!flags.front_end_https)
             hdr_out->addEntry(e->clone());
 
         break;
 
     default:
- /* pass on all other header fields */
+ /** \par default.
+ * pass on all other header fields
+ * which are NOT listed by the special Connection: header. */
+
+ if (strConnection.size()>0 && strListIsMember(&strConnection, e->name.buf(), ',')) {
+ debugs(11, 2, "'" << e->name.buf() << "' header cropped by Connection: definition");
+ return;
+ }
+
         hdr_out->addEntry(e->clone());
     }
 }

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWenA9v0ABXrfgEdxWff//3//
3+S////6YAvufbLe8PevaoA6OnZXY46l7s97VF0xJVT3s6AEohNE0YIFP0MmoZIwnpD1Gg0GTI00
HpD0j1NqDQmgEyTaEpnqj1NAANDRoDEAAAAaBomjKaapoaGhppoAGjRkxDTRoAAA0ACREjQKYmU9
TaEwTAieTKNDIaaGgaMg0A0OGhk00NMjQ0yMgyMjQyAxNGTQBkyMQwkhBGgjQTTSGnqZNU/SYFPS
PaobIT9JNND1NBoG1IkRAhUy2X8BtUfW4vtryZcwRPf1q31SMEasHzl1W97d6TmwwFbzVgbb6Oay
3HMu8yIhv3Xe+z8en+bsxnxd8+DYoBqxuzx72zfttWLQa2jFoq0qfljItin3RhltbOGLrzLF1LJi
VmraI8GVflsEI2shCMkAcRmZMD8lWW5U1cfdefHwOMQNkBdeaNyxq+ZE7y4Tnr0B2pdRRIbbG9r9
kG7uWZPA4t+KDpR214uClGTa4IcsOGyKTfKm6CrB1q5LS5Ky2L97ouyfzK/kuURtZGLq9rLLU0sM
yxZ70+KunXhzbvhfnRIMsXpI326gU3I3Njc1V7Gw00SWcD06zC8lkyzZ2662EjttjR6tFnzaJPEY
ZiRFBREwzeLT1zooszONKCyTzaV0wJgbYk+KAGiaomcHLh+OxHO6UcsYrQrjg9k9uV5vrdJi2l6U
D52gw7fHMCchRl0mmIudBCqJWVrr4sPknubVJLRJpQ6Bx5UCo92SRbAyDdf7ZU+7rGxi0mVSvugd
wSyaoxVZXwxZEJupcUzGQhCYSxpXQcN7YgjqJ6kN6rh+qXBhwERYJoyiwwykl29rymDdrOIzCYuc
YcvZ2dNeP47NXwdddK7F11xoxWLo8xPzoDrW+0QgVA85Y3avR4LDecZZK7CGt7xVuT3d/Lh8EWZo
4r/QKGwQ480QDGS48ySIEja2sO7yy2jpzQzHJP167Hj5IPGcQDGS9gxIR1ZEu09Ctqnl/e143LAr
M5GFxIHkF6RDknkhl4ijj8dy3tiIsTFsR5R/QbdZsoXW2pxxkBmgMPICOXm7+/vTNyZfFzfE5Hg1
DyJzIW6aEuhULzxCVKIhSqIrqaR2jYcaQpcvsYsksSp3IqOEHnGEfrRiZmCpMgk8JxG4YAwL9QUl
MmeP7dBFGZjrGrqfF8cWrSKGwwLhE12zPbH0sqMItH4Xa23nPYzZGaNt9t08AwBrq4EwponSDQYA
g2PVMu3IfnQWJYhoQWPz3kVuEZYbZSGgQ7pK8bdrzEpJUprNkOjcSsq3k5RiOFbh9RSrYrF6Idky
Ki4mRkZno/PBdAjHMUaY6ReMPpOsURmSPbmRzsIpgnLOPbZNGMFxORUg5jcQXb0gdFrrb4JWaGr0
mAiarpeDMEZW9WhPgYPVZkEeDM86zQ9c9hI4hgF4eMZhwZgCHiIY5UfQWWZ8i1hfsFWsF4lAEN9W
CM7fBEpIZsk/2DYbhiYxrKrRFLuNCGhbEPFU3MNG2WVeij0GCbuFNHJGZQukZjBxReX3+GkszZu1
uWYsSICo25by9uaCReONbVW4o5zuQYFtroQOppgZJCZYjbNJ3eVQpAuui6M7hp32GQxARGhPI+aI
VKczkCK0M5MSKGZgassWi9sniLpiudJRLCaxpiYj2Ra8peVmFHSLyxwMk+lrOEpqXu5+hTtUhhUf
nncSKcLxk7gsuoXUG3gpSLOv0Xj1nadyzvgGl3M0cTKj2zTBDb4gyTQzo+UOrZNsFmiJpznKgPtP
A/e/8PXOqtzV8jbG27r/6Ts/NrfV+I4C3SDUGbIms1ojW66O9mpFuy/2aOZdl2YnNxdkqTRjshLO
eE2MY5VRo+tm8wcxb3+9GegcrxPR9b1Gyemt0GBwg3QNVHBMxzCQvt9eM75NIHbeMMLEOwKolDlp
DcGsxmQnovk9qJB+Ane57ob7nliYjE4d3xt+BjYagULhXqGiIU1U4AMKJwJQjBKCUdym51Q7qoVH
STHWqMTjVDlOYbaA6jSaTOR8XTbu0SmbBRlKmQ67acBC/YSsSF8F9T/jXQqfFnGgxFxidX5l4lAQ
XsaIDHj+KJSSPO5DWES6mEzV2zGLUyPBR4XOT/lcZqbrYaHnx7+RU7xjoLj7e8wgxO8gMB7CBEae
fl8/3lnpxgXSHChLk0rdTAzlTmPsOjnHgt5cYn/EsnMgw85wNEHgP4HE8xMb8NZcXmZLAXRWL7Qh
QN5swsyAZazwjQN0dlQTrCW4NRVwh80T5KSS03omJbvtKIC43SxFp0FUTJtKoNx28Sy7s3o+ZwJS
OswPlsfWPtCqBVfXmGIRnlj3Q8qwSBztKSluEVNWKZ22IPOEDPCYUTCl2ozqlYo5Xija3MitmjtK
CJ0VriJEtOKTI3NVYcOmAgSL9W3mnBtNTaggnx1BRVOcmaKovuNyTMyGQbgkVRuOMjxL3dBXJA1x
syIuuik57ajkpVJeOhidZi7SKEAPVQJIojjTDthRTBNrfxLMY7lKjd/uRADbqTivC9S5IGizajzA
xjBtsY93UXNDtjVnpHBhObHS7o133FZ7qKzBSo4xKpn4v0PrnWZIpccx2ZNDYlD2b6Az1LL8TUhb
AwSttyuZ6XD0PTBGZMvvou09p3HYpGAatV+OTBRo0/ovYoU/S7TcIYWDM3is0sTHgNUAasp7DI1l
TVv+fC5n9W8G8wuGvG++BmSohfhSUttmtp5i3saTYMeBCPU76N6H1F8GTWjt9rsIhu3dMySK+w2+
Bko3BTzLfpvb3YyQdKDYrdJeK3Gxf9fp3ohFD8ovjLXCDhYovCTdmMScwuqw4Zh1EtxDmzxtmhVA
VQFAnbQKKq5I8z0iNfZ97nrvPDVXbs1In6f73+Q4HA2hL0eYRYmzo3748XULpPopYwR61l0q/YN7
zPTDcmEUI5+CbKJ8DsW1fcpL19OpGdklqbwnbVi8lM7vbCLPHyI+yS8x309s0CKCnr59Uvjv1iO9
YXdoYq6rGviZLsiIa5XLSWaYVo0U1eowCyju/PZFCXQTlXKX61egibqICIQUFviLxh1cMI6i4PmN
fnagiLhnGB2SZ8SEjp3CJqh9yA6vCeVkuSA+1e4seaR8Hopthynk1/CW601dpuBpsTGIHdf88WOi
6hEiKkZtBbTP6BsyZDvoF7ajU2v1vFHCULjjorOl1UhdGWD44Cw7Gzpk0D+se6t+2XkEXB1Whheo
5OGdcbMmCv8KAa8Xi6kSgxqHKtlBmKPbiR5UKT5F8yuOIzwXmBm16w4EkI7lmgHWQAx5Gq5iFDht
5iYJGA3iDBvWRy0Fx4594m4yWAnqU43WIJTxiSAtK4RLnJDreN97Pzd0Lgyi8ryyPFkOLMclyTUx
DqOXNH81iBg3ducIYri8IYODippB7ptJTfUczIZlJ+dUbhZhKKglPYc8zQo0keqKT5kwMiuwtbwQ
9eqkIzB7ySgJkTTYJJoXQ2MYNShxCJ2H34WEeuqoigf9vXuY5DoZMsktTGL+gQxDCx1CiByGJwCa
HWJqgsLE7ve/MXT/etputGbkKyLF8ExZTROho3LL5oqxUiqkVSIZbGTTIBvDF5CmvOJw5VwozTYm
zcV1w92wC3iy1EhJwxxZj3SODSmUgphYL1Igntd3BxhepQiSCZqFSqzXeMIJIox80B6yhJiV+ZAz
FmSspbl7RDJNQoUb6jsH+hCPYfJjc0AhsN0uOafWYiGNcHg3G+sg97pj9yHWKk9lt0YRjOEOkncd
OfSwimSgoH5CPU6VAuNi8fKz41pCpo6m0VTBwp0FutF/UpvsNmpZIDRxGuaWunx/+LuSKcKEh04H
t+g=
Received on Tue Jan 20 2009 - 08:57:16 MST

This archive was generated by hypermail 2.2.0 : Tue Jan 20 2009 - 12:00:08 MST