Hello,
Squid req_header ACL cannot match valid HTTP headers with empty
values. The attached patch makes it possible. I tried to limit the
changes to address this specific use case. It would be good to also
review other HttpHeader::get*() uses to check whether they mistreat
empty header fields, but that task is left for another volunteer.
Warning: Some req_header and rep_header ACLs that were [accidentally]
not matching empty headers (e.g., "^$" or ".*") will now start matching
them.
A new HttpHeader::getByNameIfPresent() method is added to be able to
detect presence of empty header fields while ACLHTTPHeaderData::match()
is adjusted to convert undefined String values into empty c-strings ("")
for ACLRegexData::match() to work.
Prior to these changes, when trying to match an empty header value with
a regex like "^$", ACLHTTPHeaderData::match() would return false because:
* HttpHeader::getStrOrList() and getByName() return an undefined String.
* String::termedBuf() returns NULL for undefined Strings; and
* ACLRegexData::match() always fails on NULL c-strings.
Thank you,
Alex.
P.S. This v3.2 patch needs a fuzz factor to apply to current trunk.
Make it possible to match empty header field values using req_header.
Warning: Some req_header and rep_header ACLs that were [accidentally] not
matching empty headers (e.g., "^$" or ".*") will now start matching them.
A new HttpHeader::getByNameIfPresent() method is added to be able to detect
presence of empty header fields while ACLHTTPHeaderData::match() is adjusted
to convert undefined String values into empty c-strings ("") for
ACLRegexData::match() to work.
Prior to these changes, when trying to match an empty header value with a
regex like "^$", ACLHTTPHeaderData::match() would return false because:
* HttpHeader::getStrOrList() and getByName() return an undefined String.
* String::termedBuf() returns NULL for undefined Strings; and
* ACLRegexData::match() always fails on NULL c-strings.
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2012-03-03 23:19:03 +0000
+++ src/HttpHeader.cc 2012-11-21 04:53:29 +0000
@@ -983,67 +983,80 @@
return s;
}
/* return a string or list of entries with the same id separated by ',' and ws */
String
HttpHeader::getStrOrList(http_hdr_type id) const
{
HttpHeaderEntry *e;
if (CBIT_TEST(ListHeadersMask, id))
return getList(id);
if ((e = findEntry(id)))
return e->value;
return String();
}
/*
- * Returns the value of the specified header.
+ * Returns the value of the specified header and/or an undefined String.
*/
String
HttpHeader::getByName(const char *name) const
{
+ String result;
+ // ignore presence: return undefined string if an empty header is present
+ (void)getByNameIfPresent(name, result);
+ return result;
+}
+
+bool
+HttpHeader::getByNameIfPresent(const char *name, String &result) const
+{
http_hdr_type id;
HttpHeaderPos pos = HttpHeaderInitPos;
HttpHeaderEntry *e;
assert(name);
/* First try the quick path */
id = httpHeaderIdByNameDef(name, strlen(name));
- if (id != -1)
- return getStrOrList(id);
-
- String result;
+ if (id != -1) {
+ if (!has(id))
+ return false;
+ result = getStrOrList(id);
+ return true;
+ }
/* Sorry, an unknown header name. Do linear search */
+ bool found = false;
while ((e = getEntry(&pos))) {
if (e->id == HDR_OTHER && e->name.caseCmp(name) == 0) {
+ found = true;
strListAdd(&result, e->value.termedBuf(), ',');
}
}
- return result;
+ return found;
}
/*
* Returns a the value of the specified list member, if any.
*/
String
HttpHeader::getByNameListMember(const char *name, const char *member, const char separator) const
{
String header;
const char *pos = NULL;
const char *item;
int ilen;
int mlen = strlen(member);
assert(name);
header = getByName(name);
String result;
=== modified file 'src/HttpHeader.h'
--- src/HttpHeader.h 2012-01-27 12:33:17 +0000
+++ src/HttpHeader.h 2012-11-21 04:53:29 +0000
@@ -230,40 +230,42 @@
/* Interface functions */
void clean();
void append(const HttpHeader * src);
void update (HttpHeader const *fresh, HttpHeaderMask const *denied_mask);
void compact();
int reset();
int parse(const char *header_start, const char *header_end);
void packInto(Packer * p) const;
HttpHeaderEntry *getEntry(HttpHeaderPos * pos) const;
HttpHeaderEntry *findEntry(http_hdr_type id) const;
int delByName(const char *name);
int delById(http_hdr_type id);
void delAt(HttpHeaderPos pos, int &headers_deleted);
void refreshMask();
void addEntry(HttpHeaderEntry * e);
void insertEntry(HttpHeaderEntry * e);
String getList(http_hdr_type id) const;
bool getList(http_hdr_type id, String *s) const;
String getStrOrList(http_hdr_type id) const;
String getByName(const char *name) const;
+ /// sets value and returns true iff a [possibly empty] named field is there
+ bool getByNameIfPresent(const char *name, String &value) const;
String getByNameListMember(const char *name, const char *member, const char separator) const;
String getListMember(http_hdr_type id, const char *member, const char separator) const;
int has(http_hdr_type id) const;
void putInt(http_hdr_type id, int number);
void putInt64(http_hdr_type id, int64_t number);
void putTime(http_hdr_type id, time_t htime);
void insertTime(http_hdr_type id, time_t htime);
void putStr(http_hdr_type 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_hdr_type id) const;
int64_t getInt64(http_hdr_type id) const;
time_t getTime(http_hdr_type id) const;
const char *getStr(http_hdr_type id) const;
const char *getLastStr(http_hdr_type id) const;
=== modified file 'src/acl/HttpHeaderData.cc'
--- src/acl/HttpHeaderData.cc 2012-02-05 06:09:46 +0000
+++ src/acl/HttpHeaderData.cc 2012-11-21 04:55:56 +0000
@@ -47,43 +47,57 @@
*
* TODO: This can be generalised by making the type of the regex_rule into a
* template parameter - so that we can use different rules types in future.
*/
ACLHTTPHeaderData::ACLHTTPHeaderData() : hdrId(HDR_BAD_HDR), regex_rule(new ACLRegexData)
{}
ACLHTTPHeaderData::~ACLHTTPHeaderData()
{
delete regex_rule;
}
bool
ACLHTTPHeaderData::match(HttpHeader* hdr)
{
if (hdr == NULL)
return false;
debugs(28, 3, "aclHeaderData::match: checking '" << hdrName << "'");
- String value = hdrId != HDR_BAD_HDR ? hdr->getStrOrList(hdrId) : hdr->getByName(hdrName.termedBuf());
-
- return regex_rule->match(value.termedBuf());
+ String value;
+ if (hdrId != HDR_BAD_HDR) {
+ if (!hdr->has(hdrId))
+ return false;
+ value = hdr->getStrOrList(hdrId);
+ } else {
+ if (!hdr->getByNameIfPresent(hdrName.termedBuf(), value))
+ return false;
+ }
+
+ // By now, we know the header is present, but:
+ // HttpHeader::get*() return an undefined String for empty header values;
+ // String::termedBuf() returns NULL for undefined Strings; and
+ // ACLRegexData::match() always fails on NULL strings.
+ // This makes it possible to detect an empty header value using regex:
+ const char *cvalue = value.defined() ? value.termedBuf() : "";
+ return regex_rule->match(cvalue);
}
wordlist *
ACLHTTPHeaderData::dump()
{
wordlist *W = NULL;
wordlistAdd(&W, hdrName.termedBuf());
wordlist * regex_dump = regex_rule->dump();
wordlistAddWl(&W, regex_dump);
wordlistDestroy(®ex_dump);
return W;
}
void
ACLHTTPHeaderData::parse()
{
char* t = strtokFile();
assert (t != NULL);
hdrName = t;
hdrId = httpHeaderIdByNameDef(hdrName.rawBuf(), hdrName.size());
