On 14.02.2017 04:22, Amos Jeffries wrote:
The problem is with proxy where the admin has configured large headers
to be allowed, and receives a Via just under the 6KB liit. Our append
pushing it over by even one byte would assert.
I am attaching a patch using SBuf instead of String for Via
accumulating, as you previously suggested. I am not
sure this is a much better solution since we have to do an
extra copy into SBuf.
> The older bbuf code
> cropping at 1KB was nasty but would not crash Squid.
I don't see the code preventing Squid from String overflow
there. The bbuf you are talking about was appended to via string
by strListAdd() without any checks...
Thanks,
Eduard.
Fixed appending Http::HdrType::VIA code duplication.
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2017-02-15 18:54:02 +0000
+++ src/HttpHeader.cc 2017-03-13 13:42:36 +0000
@@ -1,57 +1,58 @@
/*
* Copyright (C) 1996-2017 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
/* DEBUG: section 55 HTTP Header */
#include "squid.h"
#include "base/EnumIterator.h"
#include "base64.h"
#include "globals.h"
#include "http/ContentLengthInterpreter.h"
#include "HttpHdrCc.h"
#include "HttpHdrContRange.h"
#include "HttpHdrScTarget.h" // also includes HttpHdrSc.h
#include "HttpHeader.h"
#include "HttpHeaderFieldInfo.h"
#include "HttpHeaderStat.h"
#include "HttpHeaderTools.h"
#include "MemBuf.h"
#include "mgr/Registration.h"
#include "mime_header.h"
#include "profiler/Profiler.h"
#include "rfc1123.h"
+#include "sbuf/StringConvert.h"
#include "SquidConfig.h"
#include "StatHist.h"
#include "Store.h"
#include "StrList.h"
#include "TimeOrTag.h"
#include "util.h"
#include <algorithm>
/* XXX: the whole set of API managing the entries vector should be rethought
* after the parse4r-ng effort is complete.
*/
/*
* On naming conventions:
*
* HTTP/1.1 defines message-header as
*
* message-header = field-name ":" [ field-value ] CRLF
* field-name = token
* field-value = *( field-content | LWS )
*
* HTTP/1.1 does not give a name name a group of all message-headers in a message.
* Squid 1.1 seems to refer to that group _plus_ start-line as "headers".
*
* HttpHeader is an object that represents all message-headers in a message.
* HttpHeader does not manage start-line.
*
* HttpHeader is implemented as a collection of header "entries".
* An entry is a (field_id, field_name, field_value) triplet.
@@ -979,60 +980,84 @@ HttpHeader::getListMember(Http::HdrType
const char *item;
int ilen;
int mlen = strlen(member);
assert(any_registered_header(id));
header = getStrOrList(id);
String result;
while (strListGetItem(&header, separator, &item, &ilen, &pos)) {
if (strncmp(item, member, mlen) == 0 && item[mlen] == '=') {
result.append(item + mlen + 1, ilen - mlen - 1);
break;
}
}
header.clean();
return result;
}
/* test if a field is present */
int
HttpHeader::has(Http::HdrType id) const
{
assert(any_registered_header(id));
debugs(55, 9, this << " lookup for " << id);
return CBIT_TEST(mask, id);
}
void
+HttpHeader::addVia(const AnyP::ProtocolVersion &ver, const HttpHeader *from)
+{
+ // TODO: do not add Via header for messages where Squid itself
+ // generated the message (i.e., Downloader or ESI) there should be no Via header added at all.
+
+ if (Config.onoff.via) {
+ SBuf buf;
+ // RFC 7230 section 5.7.1.: protocol-name is omitted when
+ // the received protocol is HTTP.
+ if (ver.protocol > AnyP::PROTO_NONE && ver.protocol < AnyP::PROTO_UNKNOWN &&
+ ver.protocol != AnyP::PROTO_HTTP && ver.protocol != AnyP::PROTO_HTTPS)
+ buf.appendf("%s/", AnyP::ProtocolType_str[ver.protocol]);
+ buf.appendf("%d.%d %s", ver.major, ver.minor, ThisCache);
+ const HttpHeader *hdr = from ? from : this;
+ SBuf strVia = StringToSBuf(hdr->getList(Http::HdrType::VIA));
+ if (!strVia.isEmpty())
+ strVia.append(", ");
+ strVia.append(buf);
+ delById(Http::HdrType::VIA);
+ putStr(Http::HdrType::VIA, strVia.c_str());
+ }
+}
+
+void
HttpHeader::putInt(Http::HdrType id, int number)
{
assert(any_registered_header(id));
assert(Http::HeaderLookupTable.lookup(id).type == Http::HdrFieldType::ftInt); /* must be of an appropriate type */
assert(number >= 0);
addEntry(new HttpHeaderEntry(id, NULL, xitoa(number)));
}
void
HttpHeader::putInt64(Http::HdrType id, int64_t number)
{
assert(any_registered_header(id));
assert(Http::HeaderLookupTable.lookup(id).type == Http::HdrFieldType::ftInt64); /* must be of an appropriate type */
assert(number >= 0);
addEntry(new HttpHeaderEntry(id, NULL, xint64toa(number)));
}
void
HttpHeader::putTime(Http::HdrType id, time_t htime)
{
assert(any_registered_header(id));
assert(Http::HeaderLookupTable.lookup(id).type == Http::HdrFieldType::ftDate_1123); /* must be of an appropriate type */
assert(htime >= 0);
addEntry(new HttpHeaderEntry(id, NULL, mkrfc1123(htime)));
}
void
HttpHeader::putStr(Http::HdrType id, const char *str)
{
assert(any_registered_header(id));
=== modified file 'src/HttpHeader.h'
--- src/HttpHeader.h 2017-02-15 18:54:02 +0000
+++ src/HttpHeader.h 2017-03-13 12:49:09 +0000
@@ -1,41 +1,42 @@
/*
* Copyright (C) 1996-2017 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#ifndef SQUID_HTTPHEADER_H
#define SQUID_HTTPHEADER_H
+#include "anyp/ProtocolVersion.h"
#include "base/LookupTable.h"
#include "http/RegisteredHeaders.h"
/* because we pass a spec by value */
#include "HttpHeaderMask.h"
#include "mem/forward.h"
#include "sbuf/forward.h"
#include "SquidString.h"
#include <vector>
/* class forward declarations */
class HttpHdrCc;
class HttpHdrContRange;
class HttpHdrRange;
class HttpHdrSc;
class Packable;
/** Possible owners of http header */
typedef enum {
hoNone =0,
#if USE_HTCP
hoHtcpReply,
#endif
hoRequest,
hoReply,
#if USE_OPENSSL
hoErrorDetail,
#endif
hoEnd
} http_hdr_owner_type;
@@ -88,60 +89,63 @@ public:
/// \returns 1 and sets hdr_sz on success
/// \returns 0 when needs more data
/// \returns -1 on error
int parse(const char *buf, size_t buf_len, bool atEnd, size_t &hdr_sz);
void packInto(Packable * p, bool mask_sensitive_info=false) const;
HttpHeaderEntry *getEntry(HttpHeaderPos * pos) const;
HttpHeaderEntry *findEntry(Http::HdrType id) const;
int delByName(const char *name);
int delById(Http::HdrType id);
void delAt(HttpHeaderPos pos, int &headers_deleted);
void refreshMask();
void addEntry(HttpHeaderEntry * e);
void insertEntry(HttpHeaderEntry * e);
String getList(Http::HdrType id) const;
bool getList(Http::HdrType id, String *s) const;
bool conflictingContentLength() const { return conflictingContentLength_; }
String getStrOrList(Http::HdrType id) const;
String getByName(const SBuf &name) const;
String getByName(const char *name) const;
String getById(Http::HdrType id) const;
/// returns true iff a [possibly empty] field identified by id is there
/// when returning true, also sets the `result` parameter (if it is not nil)
bool getByIdIfPresent(Http::HdrType id, String *result) const;
/// returns true iff a [possibly empty] named field is there
/// when returning true, also sets the `value` parameter (if it is not nil)
bool hasNamed(const SBuf &s, String *value = 0) const;
bool hasNamed(const char *name, int namelen, String *value = 0) const;
String getByNameListMember(const char *name, const char *member, const char separator) const;
String getListMember(Http::HdrType id, const char *member, const char separator) const;
int has(Http::HdrType id) const;
+ /// Appends "this cache" information to VIA header field.
+ /// Takes the initial VIA value from "from" parameter, if provided.
+ void addVia(const AnyP::ProtocolVersion &ver, const HttpHeader *from = 0);
void putInt(Http::HdrType id, int number);
void putInt64(Http::HdrType id, int64_t number);
void putTime(Http::HdrType id, time_t htime);
void putStr(Http::HdrType id, const char *str);
void putAuth(const char *auth_scheme, const char *realm);
void putCc(const HttpHdrCc * cc);
void putContRange(const HttpHdrContRange * cr);
void putRange(const HttpHdrRange * range);
void putSc(HttpHdrSc *sc);
void putWarning(const int code, const char *const text); ///< add a Warning header
void putExt(const char *name, const char *value);
int getInt(Http::HdrType id) const;
int64_t getInt64(Http::HdrType id) const;
time_t getTime(Http::HdrType id) const;
const char *getStr(Http::HdrType id) const;
const char *getLastStr(Http::HdrType id) const;
HttpHdrCc *getCc() const;
HttpHdrRange *getRange() const;
HttpHdrSc *getSc() const;
HttpHdrContRange *getContRange() const;
const char *getAuth(Http::HdrType id, const char *auth_scheme) const;
ETag getETag(Http::HdrType id) const;
TimeOrTag getTimeOrTag(Http::HdrType id) const;
int hasListMember(Http::HdrType id, const char *member, const char separator) const;
int hasByNameListMember(const char *name, const char *member, const char separator) const;
void removeHopByHopEntries();
inline bool chunked() const; ///< whether message uses chunked Transfer-Encoding
/* protected, do not use these, use interface functions instead */
std::vector<HttpHeaderEntry *> entries; /**< parsed fields in raw format */
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2017-03-01 04:52:46 +0000
+++ src/client_side_reply.cc 2017-03-13 12:48:29 +0000
@@ -1568,73 +1568,62 @@ clientReplyContext::buildReplyHeader()
} else if (reply->bodySize(request->method) < 0 && !maySendChunkedReply) {
debugs(88, 3, "clientBuildReplyHeader: can't keep-alive, unknown body size" );
request->flags.proxyKeepalive = false;
} else if (fdUsageHigh()&& !request->flags.mustKeepalive) {
debugs(88, 3, "clientBuildReplyHeader: Not many unused FDs, can't keep-alive");
request->flags.proxyKeepalive = false;
} else if (request->flags.sslBumped && !reply->persistent()) {
// We do not really have to close, but we pretend we are a tunnel.
debugs(88, 3, "clientBuildReplyHeader: bumped reply forces close");
request->flags.proxyKeepalive = false;
} else if (request->pinnedConnection() && !reply->persistent()) {
// The peer wants to close the pinned connection
debugs(88, 3, "pinned reply forces close");
request->flags.proxyKeepalive = false;
} else if (http->getConn()) {
ConnStateData * conn = http->getConn();
if (!Comm::IsConnOpen(conn->port->listenConn)) {
// The listening port closed because of a reconfigure
debugs(88, 3, "listening port closed");
request->flags.proxyKeepalive = false;
}
}
// Decide if we send chunked reply
if (maySendChunkedReply && reply->bodySize(request->method) < 0) {
debugs(88, 3, "clientBuildReplyHeader: chunked reply");
request->flags.chunkedReply = true;
hdr->putStr(Http::HdrType::TRANSFER_ENCODING, "chunked");
}
- /* Append VIA */
- if (Config.onoff.via) {
- LOCAL_ARRAY(char, bbuf, MAX_URL + 32);
- String strVia;
- hdr->getList(Http::HdrType::VIA, &strVia);
- snprintf(bbuf, MAX_URL + 32, "%d.%d %s",
- reply->sline.version.major,
- reply->sline.version.minor,
- ThisCache);
- strListAdd(&strVia, bbuf, ',');
- hdr->delById(Http::HdrType::VIA);
- hdr->putStr(Http::HdrType::VIA, strVia.termedBuf());
- }
+ hdr->addVia(reply->sline.version);
+
/* Signal keep-alive or close explicitly */
hdr->putStr(Http::HdrType::CONNECTION, request->flags.proxyKeepalive ? "keep-alive" : "close");
#if ADD_X_REQUEST_URI
/*
* Knowing the URI of the request is useful when debugging persistent
* connections in a client; we cannot guarantee the order of http headers,
* but X-Request-URI is likely to be the very last header to ease use from a
* debugger [hdr->entries.count-1].
*/
hdr->putStr(Http::HdrType::X_REQUEST_URI,
http->memOjbect()->url ? http->memObject()->url : http->uri);
#endif
/* Surrogate-Control requires Surrogate-Capability from upstream to pass on */
if ( hdr->has(Http::HdrType::SURROGATE_CONTROL) ) {
if (!request->header.has(Http::HdrType::SURROGATE_CAPABILITY)) {
hdr->delById(Http::HdrType::SURROGATE_CONTROL);
}
/* TODO: else case: drop any controls intended specifically for our surrogate ID */
}
httpHdrMangleList(hdr, request, http->al, ROR_REPLY);
}
void
clientReplyContext::cloneReply()
{
assert(reply == NULL);
=== modified file 'src/http.cc'
--- src/http.cc 2017-03-03 22:15:10 +0000
+++ src/http.cc 2017-03-13 12:48:29 +0000
@@ -1768,71 +1768,61 @@ HttpStateData::httpBuildRequestHeader(Ht
/* use our IMS header if the cached entry has Last-Modified time */
if (request->lastmod > -1)
hdr_out->putTime(Http::HdrType::IF_MODIFIED_SINCE, request->lastmod);
// Add our own If-None-Match field if the cached entry has a strong ETag.
// copyOneHeaderFromClientsideRequestToUpstreamRequest() adds client ones.
if (request->etag.size() > 0) {
hdr_out->addEntry(new HttpHeaderEntry(Http::HdrType::IF_NONE_MATCH, NULL,
request->etag.termedBuf()));
}
bool we_do_ranges = decideIfWeDoRanges (request);
String strConnection (hdr_in->getList(Http::HdrType::CONNECTION));
while ((e = hdr_in->getEntry(&pos)))
copyOneHeaderFromClientsideRequestToUpstreamRequest(e, strConnection, request, hdr_out, we_do_ranges, flags);
/* Abstraction break: We should interpret multipart/byterange responses
* into offset-length data, and this works around our inability to do so.
*/
if (!we_do_ranges && request->multipartRangeRequest()) {
/* don't cache the result */
request->flags.cachable = false;
/* pretend it's not a range request */
request->ignoreRange("want to request the whole object");
request->flags.isRanged = false;
}
- /* append Via */
- if (Config.onoff.via) {
- String strVia;
- strVia = hdr_in->getList(Http::HdrType::VIA);
- snprintf(bbuf, BBUF_SZ, "%d.%d %s",
- request->http_ver.major,
- request->http_ver.minor, ThisCache);
- strListAdd(&strVia, bbuf, ',');
- hdr_out->putStr(Http::HdrType::VIA, strVia.termedBuf());
- strVia.clean();
- }
+ hdr_out->addVia(request->http_ver, hdr_in);
if (request->flags.accelerated) {
/* Append Surrogate-Capabilities */
String strSurrogate(hdr_in->getList(Http::HdrType::SURROGATE_CAPABILITY));
#if USE_SQUID_ESI
snprintf(bbuf, BBUF_SZ, "%s=\"Surrogate/1.0 ESI/1.0\"", Config.Accel.surrogate_id);
#else
snprintf(bbuf, BBUF_SZ, "%s=\"Surrogate/1.0\"", Config.Accel.surrogate_id);
#endif
strListAdd(&strSurrogate, bbuf, ',');
hdr_out->putStr(Http::HdrType::SURROGATE_CAPABILITY, strSurrogate.termedBuf());
}
/** \pre Handle X-Forwarded-For */
if (strcmp(opt_forwarded_for, "delete") != 0) {
String strFwd = hdr_in->getList(Http::HdrType::X_FORWARDED_FOR);
// if we cannot double strFwd size, then it grew past 50% of the limit
if (!strFwd.canGrowBy(strFwd.size())) {
// There is probably a forwarding loop with Via detection disabled.
// If we do nothing, String will assert on overflow soon.
// TODO: Terminate all transactions with huge XFF?
strFwd = "error";
static int warnedCount = 0;
if (warnedCount++ < 100) {
const SBuf url(entry ? SBuf(entry->url()) : request->effectiveRequestUri());
debugs(11, DBG_IMPORTANT, "Warning: likely forwarding loop with " << url);
}
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
