You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2014/11/11 18:49:49 UTC
trafficserver git commit: TS-3155: Adding value_get_index to test for
presence of value in MIME header field. Using it in the keep alive checks.
This closes #139 pull request.
Repository: trafficserver
Updated Branches:
refs/heads/master 358cee17b -> 378a5b53f
TS-3155: Adding value_get_index to test for presence of value in MIME header
field. Using it in the keep alive checks. This closes #139 pull request.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/378a5b53
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/378a5b53
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/378a5b53
Branch: refs/heads/master
Commit: 378a5b53f81ca7ab27c5ac9693734af0bed01c9d
Parents: 358cee1
Author: shinrich <sh...@network-geographics.com>
Authored: Fri Nov 7 11:31:03 2014 -0600
Committer: Alan M. Carroll <so...@yahoo-inc.com>
Committed: Tue Nov 11 11:48:34 2014 -0600
----------------------------------------------------------------------
CHANGES | 2 +
proxy/hdrs/HdrTest.cc | 23 +++++++++++
proxy/hdrs/HdrUtils.h | 10 ++---
proxy/hdrs/MIME.cc | 39 ++++++++++++++++---
proxy/hdrs/MIME.h | 31 ++++++++++++++-
proxy/http/HttpTransact.cc | 84 ++++++++++++-----------------------------
6 files changed, 117 insertions(+), 72 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index ef86a60..8895bfb 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 5.2.0
+ *) [TS-3155] Added value_get_index to MimeField.
+
*) [TS-3184] spdy window_update not triggered correctly
*) [TS-3024] Build with OPENSSL_NO_SSL_INTERN
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/proxy/hdrs/HdrTest.cc
----------------------------------------------------------------------
diff --git a/proxy/hdrs/HdrTest.cc b/proxy/hdrs/HdrTest.cc
index 637a836..23ae826 100644
--- a/proxy/hdrs/HdrTest.cc
+++ b/proxy/hdrs/HdrTest.cc
@@ -537,6 +537,7 @@ HdrTest::test_mime()
"ACCEPT\r\n"
"foo: bar\r\n"
"foo: argh\r\n"
+ "foo: three, four\r\n"
"word word: word \r\n"
"accept: \"fazzle, dazzle\"\r\n"
"accept: 1, 2, 3, 4, 5, 6, 7, 8\r\n"
@@ -610,11 +611,33 @@ HdrTest::test_mime()
// TODO: Do we need to check the "count" returned?
cc_field->value_get_comma_list(&slist); // FIX: correct usage?
+ if (cc_field->value_get_index("Private", 7) < 0) {
+ printf("Failed: value_get_index of Cache-Control did not find private");
+ return (failures_to_status("test_mime", 1));
+ }
+ if (cc_field->value_get_index("Bogus", 5) >= 0) {
+ printf("Failed: value_get_index of Cache-Control incorrectly found bogus");
+ return (failures_to_status("test_mime", 1));
+ }
+ if (hdr.value_get_index("foo", 3, "three", 5) < 0) {
+ printf("Failed: value_get_index of foo did not find three");
+ return (failures_to_status("test_mime", 1));
+ }
+ if (hdr.value_get_index("foo", 3, "bar", 3) < 0) {
+ printf("Failed: value_get_index of foo did not find bar");
+ return (failures_to_status("test_mime", 1));
+ }
+ if (hdr.value_get_index("foo", 3, "Bogus", 5) >= 0) {
+ printf("Failed: value_get_index of foo incorrectly found bogus");
+ return (failures_to_status("test_mime", 1));
+ }
+
mime_parser_clear(&parser);
hdr.print(NULL, 0, NULL, NULL);
printf("\n");
+
obj_describe((HdrHeapObjImpl *) (hdr.m_mime), true);
hdr.fields_clear();
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/proxy/hdrs/HdrUtils.h
----------------------------------------------------------------------
diff --git a/proxy/hdrs/HdrUtils.h b/proxy/hdrs/HdrUtils.h
index fd26863..8fc7108 100644
--- a/proxy/hdrs/HdrUtils.h
+++ b/proxy/hdrs/HdrUtils.h
@@ -47,7 +47,7 @@ HdrCsvIter(const char s = ','):m_value_start(NULL),
m_value_len(0), m_bytes_consumed(0), m_follow_dups(false),
m_csv_start(NULL), m_csv_len(0), m_csv_end(NULL), m_csv_index(0), m_cur_field(NULL), m_separator(s) {
}
- const char *get_first(MIMEField * m, int *len, bool follow_dups = true);
+ const char *get_first(const MIMEField * m, int *len, bool follow_dups = true);
const char *get_next(int *len);
const char *get_current(int *len);
@@ -83,16 +83,16 @@ private:
int m_csv_len;
const char *m_csv_end;
int m_csv_index;
- MIMEField *m_cur_field;
+ const MIMEField *m_cur_field;
// for the Cookie/Set-cookie headers, the separator is ';'
const char m_separator;
- void field_init(MIMEField * m);
+ void field_init(const MIMEField * m);
};
inline void
-HdrCsvIter::field_init(MIMEField * m)
+HdrCsvIter::field_init(const MIMEField * m)
{
m_cur_field = m;
m_value_start = m->m_ptr_value;
@@ -101,7 +101,7 @@ HdrCsvIter::field_init(MIMEField * m)
}
inline const char *
-HdrCsvIter::get_first(MIMEField * m, int *len, bool follow_dups)
+HdrCsvIter::get_first(const MIMEField * m, int *len, bool follow_dups)
{
field_init(m);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/proxy/hdrs/MIME.cc
----------------------------------------------------------------------
diff --git a/proxy/hdrs/MIME.cc b/proxy/hdrs/MIME.cc
index 69acaa6..6441841 100644
--- a/proxy/hdrs/MIME.cc
+++ b/proxy/hdrs/MIME.cc
@@ -636,14 +636,14 @@ mime_init()
if (init) {
init = 0;
-
+
hdrtoken_init();
day_names_dfa = new DFA;
day_names_dfa->compile(day_names, SIZEOF(day_names), RE_CASE_INSENSITIVE);
-
+
month_names_dfa = new DFA;
month_names_dfa->compile(month_names, SIZEOF(month_names), RE_CASE_INSENSITIVE);
-
+
MIME_FIELD_ACCEPT = hdrtoken_string_to_wks("Accept");
MIME_FIELD_ACCEPT_CHARSET = hdrtoken_string_to_wks("Accept-Charset");
MIME_FIELD_ACCEPT_ENCODING = hdrtoken_string_to_wks("Accept-Encoding");
@@ -1722,6 +1722,33 @@ mime_field_name_set(HdrHeap *heap, MIMEHdrImpl */* mh ATS_UNUSED */, MIMEField *
}
}
+int
+MIMEField::value_get_index(char const *value, int length) const {
+ int retval = -1;
+
+ // if field doesn't support commas and there is just one instance, just compare the value
+ if (!this->supports_commas() && !this->has_dups()) {
+ if (this->m_len_value == length &&
+ strncasecmp(value, this->m_ptr_value, length) == 0)
+ retval = 0;
+ } else {
+ HdrCsvIter iter;
+ int tok_len;
+ const char *tok = iter.get_first(this, &tok_len);
+ int index = 0;
+ while (tok) {
+ if (tok_len == length && strncasecmp(tok, value, length) == 0) {
+ retval = index;
+ break;
+ } else {
+ index++;
+ }
+ tok = iter.get_next(&tok_len);
+ }
+ }
+ return retval;
+}
+
const char *
mime_field_value_get(const MIMEField *field, int *length)
{
@@ -2204,7 +2231,7 @@ MIMEField* MIMEHdr::get_host_port_values(
host = b;
}
}
-
+
if (host) {
if (host_ptr) *host_ptr = host._ptr;
if (host_len) *host_len = static_cast<int>(host._size);
@@ -2399,7 +2426,7 @@ mime_scanner_get(MIMEScanner *S,
mime_scanner_append(S, *raw_input_s, data_size);
data_size = 0; // Don't append again.
}
- }
+ }
if (data_size && S->m_line_length) {
// If we're already accumulating, continue to do so if we have data.
@@ -2419,7 +2446,7 @@ mime_scanner_get(MIMEScanner *S,
*output_shares_raw_input = true;
}
}
-
+
*raw_input_s = raw_input_c; // mark input consumed.
return zret;
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/proxy/hdrs/MIME.h
----------------------------------------------------------------------
diff --git a/proxy/hdrs/MIME.h b/proxy/hdrs/MIME.h
index 1973a9a..127da61 100644
--- a/proxy/hdrs/MIME.h
+++ b/proxy/hdrs/MIME.h
@@ -135,6 +135,20 @@ struct MIMEField
const char *name_get(int *length) const;
+ /** Find the index of the value in the multi-value field.
+
+ If @a value is one of the values in this field return the
+ 0 based index of it in the list of values. If the field is
+ not multivalued the index will always be zero if found.
+ Otherwise return -1 if the @a value is not found.
+
+ @note The most common use of this is to check for the presence of a specific
+ value in a comma enabled field.
+
+ @return The index of @a value.
+ */
+ int value_get_index(char const *value, int length) const;
+
const char *value_get(int *length) const;
int32_t value_get_int() const;
uint32_t value_get_uint() const;
@@ -154,7 +168,7 @@ struct MIMEField
// Other separators (e.g. ';' in Set-cookie/Cookie) are also possible
void value_append(HdrHeap * heap, MIMEHdrImpl * mh, const char *value,
int length, bool prepend_comma = false, const char separator = ',');
- int has_dups();
+ int has_dups() const;
};
struct MIMEFieldBlockImpl:public HdrHeapObjImpl
@@ -844,7 +858,7 @@ MIMEField::value_append(HdrHeap * heap, MIMEHdrImpl * mh, const char *value,
}
inline int
-MIMEField::has_dups()
+MIMEField::has_dups() const
{
return (m_next_dup != NULL);
}
@@ -905,6 +919,7 @@ public:
int parse(MIMEParser * parser, const char **start, const char *end, bool must_copy_strs, bool eof);
+ int value_get_index(const char *name, int name_length, const char *value, int value_length);
const char *value_get(const char *name, int name_length, int *value_length);
int32_t value_get_int(const char *name, int name_length);
uint32_t value_get_uint(const char *name, int name_length);
@@ -1189,6 +1204,18 @@ MIMEHdr::parse(MIMEParser * parser, const char **start, const char *end, bool mu
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
+inline int
+MIMEHdr::value_get_index(const char *name, int name_length, const char *value, int value_length)
+{
+ MIMEField *field = field_find(name, name_length);
+ if (field)
+ return field->value_get_index(value, value_length);
+ else
+ return -1;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
inline const char *
MIMEHdr::value_get(const char *name, int name_length, int *value_length_return)
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/378a5b53/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index e0d0df1..662d651 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -88,44 +88,10 @@ is_header_keep_alive(const HTTPVersion & http_version, const HTTPVersion & reque
// *unknown_tokens = false;
if (con_hdr) {
- int val_len;
- const char *val;
-
- if (!con_hdr->has_dups()) { // try fastpath first
- val = con_hdr->value_get(&val_len);
- if (ptr_len_casecmp(val, val_len, "keep-alive", 10) == 0) {
- con_token = CON_TOKEN_KEEP_ALIVE;
- } else if (ptr_len_casecmp(val, val_len, "close", 5) == 0) {
- con_token = CON_TOKEN_CLOSE;
- }
- }
-
- if (con_token == CON_TOKEN_NONE) {
- HdrCsvIter iter;
-
- val = iter.get_first(con_hdr, &val_len);
-
- while (val) {
- if (ptr_len_casecmp(val, val_len, "keep-alive", 10) == 0) {
- con_token = CON_TOKEN_KEEP_ALIVE;
- /*
- if (!*unknown_tokens) {
- *unknown_tokens = (iter->get_next(&val_len) == NULL);
- } */
- break;
- } else if (ptr_len_casecmp(val, val_len, "close", 5) == 0) {
- con_token = CON_TOKEN_CLOSE;
- /*
- if (!*unknown_tokens) {
- *unknown_tokens = (iter->get_next(&val_len) == NULL);
- } */
- break;
- } else {
- // *unknown_tokens = true;
- }
- val = iter.get_next(&val_len);
- }
- }
+ if (con_hdr->value_get_index("keep-alive", 10) >= 0)
+ con_token = CON_TOKEN_KEEP_ALIVE;
+ else if (con_hdr->value_get_index("close", 5) >= 0)
+ con_token = CON_TOKEN_CLOSE;
}
if (HTTPVersion(1, 0) == http_version) {
@@ -528,7 +494,7 @@ how_to_open_connection(HttpTransact::State* s)
int port = url->port_get();
if (port != url_canonicalize_port(URL_TYPE_HTTP, 0)) {
char *buf = (char *)alloca(host_len + 15);
- memcpy(buf, host, host_len);
+ memcpy(buf, host, host_len);
host_len += snprintf(buf + host_len, 15, ":%d", port);
s->hdr_info.server_request.value_set(MIME_FIELD_HOST, MIME_LEN_HOST, buf, host_len);
} else {
@@ -640,7 +606,7 @@ HttpTransact::HandleBlindTunnel(State* s)
url_remap_success = true;
} else if (url_remap_mode == URL_REMAP_DEFAULT || url_remap_mode == URL_REMAP_ALL) {
// TODO: take a look at this
- // This probably doesn't work properly, since request_url_remap() is broken.
+ // This probably doesn't work properly, since request_url_remap() is broken.
url_remap_success = request_url_remap(s, &s->hdr_info.client_request, &remap_redirect);
}
// We must have mapping or we will self loop since the this
@@ -756,7 +722,7 @@ HttpTransact::perform_accept_encoding_filtering(State* s)
void
HttpTransact::StartRemapRequest(State* s)
{
-
+
if (s->api_skip_all_remapping) {
Debug ("http_trans", "API request to skip remapping");
@@ -766,7 +732,7 @@ HttpTransact::StartRemapRequest(State* s)
TRANSACT_RETURN(SM_ACTION_POST_REMAP_SKIP, HttpTransact::HandleRequest);
}
-
+
DebugTxn("http_trans", "START HttpTransact::StartRemapRequest");
/**
@@ -842,7 +808,7 @@ HttpTransact::EndRemapRequest(State* s)
int host_len;
const char *host = incoming_request->host_get(&host_len);
DebugTxn("http_trans","EndRemapRequest host is %.*s", host_len,host);
-
+
////////////////////////////////////////////////////////////////
// if we got back a URL to redirect to, vector the user there //
////////////////////////////////////////////////////////////////
@@ -956,9 +922,9 @@ done:
/*
if s->reverse_proxy == false, we can assume remapping failed in some way
-however-
- If an API setup a tunnel to fake the origin or proxy's response we will
+ If an API setup a tunnel to fake the origin or proxy's response we will
continue to handle the request (as this was likely the plugin author's intent)
-
+
otherwise, 502/404 the request right now. /eric
*/
if (!s->reverse_proxy && s->state_machine->plugin_tunnel_type == HTTP_NO_PLUGIN_TUNNEL) {
@@ -1362,7 +1328,7 @@ HttpTransact::HandleRequest(State* s)
// origin server. Ignore the no_dns_just_forward_to_parent setting.
// we need to see if the hostname is an
// ip address since the parent selection code result
- // could change as a result of this ip address
+ // could change as a result of this ip address
IpEndpoint addr;
ats_ip_pton(s->server_info.name, &addr);
if (ats_is_ip(&addr)) {
@@ -1476,9 +1442,9 @@ HttpTransact::HandleApiErrorJump(State* s)
s->hdr_info.client_response.fields_clear();
s->source = SOURCE_INTERNAL;
}
-
- /**
- The API indicated an error. Lets use a >=400 error from the state (if one's set) or fallback to a
+
+ /**
+ The API indicated an error. Lets use a >=400 error from the state (if one's set) or fallback to a
generic HTTP/1.X 500 INKApi Error
**/
if ( s->http_return_code && s->http_return_code >= HTTP_STATUS_BAD_REQUEST ) {
@@ -1489,7 +1455,7 @@ HttpTransact::HandleApiErrorJump(State* s)
else {
build_response(s, &s->hdr_info.client_response, s->client_info.http_version,
HTTP_STATUS_INTERNAL_SERVER_ERROR, "INKApi Error");
- }
+ }
TRANSACT_RETURN(SM_ACTION_INTERNAL_CACHE_NOOP, NULL);
return;
@@ -3130,9 +3096,9 @@ HttpTransact::HandleICPLookup(State* s)
// TODO in this case we should go to the miss case
// just a little shy about using goto's, that's all.
ink_release_assert(
- (s->icp_info.port != s->client_info.port) ||
+ (s->icp_info.port != s->client_info.port) ||
(ats_ip_addr_cmp(&s->icp_info.addr.sa, &Machine::instance()->ip.sa) != 0)
- );
+ );
// Since the ICPDNSLookup is not called, these two
// values are not initialized.
@@ -3708,7 +3674,7 @@ void
HttpTransact::delete_server_rr_entry(State* s, int max_retries)
{
char addrbuf[INET6_ADDRSTRLEN];
-
+
DebugTxn("http_trans", "[%d] failed to connect to %s", s->current.attempts,
ats_ip_ntop(&s->current.server->addr.sa, addrbuf, sizeof(addrbuf)));
DebugTxn("http_trans", "[delete_server_rr_entry] marking rr entry " "down and finding next one");
@@ -6014,7 +5980,7 @@ HttpTransact::is_response_cacheable(State* s, HTTPHdr* request, HTTPHdr* respons
// If the use_client_target_addr is specified but the client
// specified OS addr does not match any of trafficserver's looked up
// host addresses, do not allow cache. This may cause DNS cache poisoning
- // of other trafficserver clients. The flag is set in the
+ // of other trafficserver clients. The flag is set in the
// process_host_db_info method
if (!s->dns_info.lookup_validated
&& s->client_info.is_transparent) {
@@ -8130,9 +8096,9 @@ HttpTransact::build_error_response(State *s, HTTPStatus status_code, const char
if (s->http_config_param->errors_log_error_pages && status_code >= HTTP_STATUS_BAD_REQUEST) {
char ip_string[INET6_ADDRSTRLEN];
- Log::error("RESPONSE: sent %s status %d (%s) for '%s'",
- ats_ip_ntop(&s->client_info.addr.sa, ip_string, sizeof(ip_string)),
- status_code,
+ Log::error("RESPONSE: sent %s status %d (%s) for '%s'",
+ ats_ip_ntop(&s->client_info.addr.sa, ip_string, sizeof(ip_string)),
+ status_code,
reason_phrase,
(url_string ? url_string : "<none>"));
}
@@ -8197,7 +8163,7 @@ HttpTransact::build_redirect_response(State* s)
s->internal_msg_buffer = body_factory->fabricate_with_old_api_build_va("redirect#moved_temporarily", s, 8192,
&s->internal_msg_buffer_size,
body_language, sizeof(body_language),
- body_type, sizeof(body_type),
+ body_type, sizeof(body_type),
"%s <a href=\"%s\">%s</a>. %s.",
"The document you requested is now",
new_url, new_url,
@@ -8496,7 +8462,7 @@ HttpTransact::client_result_stat(State* s, ink_hrtime total_time, ink_hrtime req
default: break;
}
}
-
+
// Increment the completed connection count
HTTP_INCREMENT_TRANS_STAT(http_completed_requests_stat);