On 09.02.2017 20:19, Amos Jeffries wrote:
> Since Via is a list header we should be able to just append a new Via
> header to the header list with putStr. No need to use getList, String,
> delById to inject on the end of existing Via string.
Doing so will change the Via generation way currently used. Instead of
generating a single Via header with comma-separated value list Squid
would produce several Via header fields. Though this is equivalent from
the RFC point of view, I would prefer to leave these things "as is" for now.
> Please also take the opportunity to fix a long standing protocol
> violation bug in Via header produced by Squid:
> The version part of the header is only supposed to elide the protocol
> label if the that label would be "HTTP/" (ie. for messages received via
> HTTP and HTTPS).
>
> eg. for ICY replies:
> Via: ICY/1.1 squid
>
> eg. for FTP gatewayed replies (or relayed requests):
> Via: FTP/2 squid
Done.
> After those calls that require String are gone please assemble the value
> in an SBuf instead of static char* buffer.
Done.
> Also, for messages where Squid itself generated the message (ie
> Downloader or ESI) there should be no Via header added at all.
Marked as TODO.
Eduard.
Fixed appending Http::HdrType::VIA code duplication.
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2017-01-25 22:29:03 +0000
+++ src/HttpHeader.cc 2017-02-11 19:30:36 +0000
@@ -976,60 +976,83 @@
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);
+ String strVia;
+ const HttpHeader *hdr = from ? from : this;
+ hdr->getList(Http::HdrType::VIA, &strVia);
+ strListAdd(&strVia, buf.c_str(), ',');
+ delById(Http::HdrType::VIA);
+ putStr(Http::HdrType::VIA, strVia.termedBuf());
+ }
+}
+
+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-01-01 00:12:22 +0000
+++ src/HttpHeader.h 2017-02-11 16:53:40 +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;
@@ -86,60 +87,63 @@
int parse(const char *header_start, size_t len);
/// Parses headers stored in a buffer.
/// \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;
/// sets value and returns true iff a [possibly empty] field identified by id is there
bool getByIdIfPresent(Http::HdrType id, String &result) const;
/// sets value and returns true iff a [possibly empty] named field is there
bool getByNameIfPresent(const SBuf &s, String &value) const;
bool getByNameIfPresent(const char *name, int namelen, String &value) 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-01-01 00:12:22 +0000
+++ src/client_side_reply.cc 2017-02-11 16:53:40 +0000
@@ -1567,73 +1567,62 @@
} 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-01-01 00:12:22 +0000
+++ src/http.cc 2017-02-11 16:53:40 +0000
@@ -1786,71 +1786,61 @@
/* 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
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
