You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2010/09/29 18:37:01 UTC
svn commit: r1002726 - in /trafficserver/traffic/trunk/proxy: hdrs/MIME.h
http2/HttpTransact.cc http2/HttpTransactCache.cc http2/HttpTransactHeaders.cc
Author: zwoop
Date: Wed Sep 29 16:37:00 2010
New Revision: 1002726
URL: http://svn.apache.org/viewvc?rev=1002726&view=rev
Log:
More cleanup for dealing with Age: headers / values properly.
I changed the semantics to have a negative value mean overflow,
and keep using time_t through the code. This has the bonus of
making 64-bit boxes properly handle very large Age: values without
getting overflows.
Modified:
trafficserver/traffic/trunk/proxy/hdrs/MIME.h
trafficserver/traffic/trunk/proxy/http2/HttpTransact.cc
trafficserver/traffic/trunk/proxy/http2/HttpTransactCache.cc
trafficserver/traffic/trunk/proxy/http2/HttpTransactHeaders.cc
Modified: trafficserver/traffic/trunk/proxy/hdrs/MIME.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/hdrs/MIME.h?rev=1002726&r1=1002725&r2=1002726&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/hdrs/MIME.h (original)
+++ trafficserver/traffic/trunk/proxy/hdrs/MIME.h Wed Sep 29 16:37:00 2010
@@ -912,7 +912,7 @@ public:
// Other separators (e.g. ';' in Set-cookie/Cookie) are also possible
void field_value_append(MIMEField * field,
const char *value, int value_length, bool prepend_comma = false, const char separator = ',');
- uint32 get_age();
+ time_t get_age();
int64 get_content_length();
time_t get_date();
time_t get_expires();
@@ -933,7 +933,7 @@ public:
void set_cooked_cc_need_revalidate_once();
void unset_cooked_cc_need_revalidate_once();
- void set_age(uint32 value);
+ void set_age(time_t value);
void set_content_length(int64 value);
void set_date(time_t value);
void set_expires(time_t value);
@@ -1322,16 +1322,18 @@ MIMEHdr::value_append(const char *name,
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
-
-inline uint32
+inline time_t
MIMEHdr::get_age()
{
int64 age = value_get_int64(MIME_FIELD_AGE, MIME_LEN_AGE);
- if (age < 0)
+ if (age < 0) // We should ignore negative Age: values
return 0;
- return (age > INT_MAX ? (uint32)INT_MAX + 1 : age);
+ if ((4 == sizeof(time_t)) && (age > INT_MAX)) // Overflow
+ return -1;
+
+ return age;
}
/*-------------------------------------------------------------------------
@@ -1494,12 +1496,12 @@ MIMEHdr::unset_cooked_cc_need_revalidate
-------------------------------------------------------------------------*/
inline void
-MIMEHdr::set_age(uint32 value)
+MIMEHdr::set_age(time_t value)
{
- if (value > INT_MAX)
+ if (value < 0)
value_set_uint(MIME_FIELD_AGE, MIME_LEN_AGE, (uint32)INT_MAX + 1);
else
- value_set_uint(MIME_FIELD_AGE, MIME_LEN_AGE, value);
+ value_set_uint(MIME_FIELD_AGE, MIME_LEN_AGE, value); // SHOULD BE int64
}
/*-------------------------------------------------------------------------
Modified: trafficserver/traffic/trunk/proxy/http2/HttpTransact.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http2/HttpTransact.cc?rev=1002726&r1=1002725&r2=1002726&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http2/HttpTransact.cc (original)
+++ trafficserver/traffic/trunk/proxy/http2/HttpTransact.cc Wed Sep 29 16:37:00 2010
@@ -4855,7 +4855,7 @@ HttpTransact::merge_and_update_headers_f
// Hack fix. If the server sends back
// a 304 without a Date Header, use the current time
// as the new Date value in the header to be cached.
- int64 date_value = s->hdr_info.server_response.get_date(); // was time_t but it's signed int on some OSes
+ time_t date_value = s->hdr_info.server_response.get_date();
HTTPHdr *cached_hdr = s->cache_info.object_store.response_get();
if (date_value <= 0) {
@@ -4865,10 +4865,15 @@ HttpTransact::merge_and_update_headers_f
// If the cached response has an Age: we should update it
// We could use calculate_document_age but my guess is it's overkill
// Just use 'now' - 304's Date: + Age: (response's Age: if there)
- date_value = max(s->current.now - date_value, (int64)0);
- if (s->hdr_info.server_response.presence(MIME_PRESENCE_AGE))
- date_value += s->hdr_info.server_response.get_age();
- cached_hdr->set_age(date_value);
+ date_value = max(s->current.now - date_value, (ink_time_t)0);
+ if (s->hdr_info.server_response.presence(MIME_PRESENCE_AGE)) {
+ time_t new_age = s->hdr_info.server_response.get_age();
+
+ if (new_age >= 0)
+ cached_hdr->set_age(date_value + new_age);
+ else
+ cached_hdr->set_age(-1); // Overflow
+ }
delete_warning_value(cached_hdr, HTTP_WARNING_CODE_REVALIDATION_FAILED);
}
@@ -6200,18 +6205,19 @@ HttpTransact::is_stale_cache_response_re
}
// See how old the document really is. We don't want create a
// stale content museum of doucments that are no longer available
- int current_age = HttpTransactHeaders::calculate_document_age(s->cache_info.object_read->request_sent_time_get(),
- s->cache_info.object_read->response_received_time_get(),
- cached_response,
- cached_response->get_date(),
- s->current.now);
- if (current_age > s->http_config_param->cache_max_stale_age) {
+ time_t current_age = HttpTransactHeaders::calculate_document_age(s->cache_info.object_read->request_sent_time_get(),
+ s->cache_info.object_read->response_received_time_get(),
+ cached_response,
+ cached_response->get_date(),
+ s->current.now);
+ // Negative age is overflow
+ if ((current_age < 0) || (current_age > s->http_config_param->cache_max_stale_age)) {
Debug("http_trans", "[is_stale_cache_response_returnable] " "document age is too large %d", current_age);
return false;
}
// If the stale document requires authorization, we can't return it either.
- Authentication_t
- auth_needed = AuthenticationNeeded(s->http_config_param, &s->hdr_info.client_request, cached_response);
+ Authentication_t auth_needed = AuthenticationNeeded(s->http_config_param, &s->hdr_info.client_request, cached_response);
+
if (auth_needed != AUTHENTICATION_SUCCESS) {
Debug("http_trans", "[is_stale_cache_response_returnable] " "authorization prevent serving stale");
return false;
@@ -7730,8 +7736,8 @@ HttpTransact::what_is_document_freshness
// These aren't used.
//HTTPValCacheControl *cc;
//const char *cc_val;
- int fresh_limit, current_age;
- ink_time_t response_date;
+ int fresh_limit;
+ ink_time_t current_age, response_date;;
uint32 cc_mask, cooked_cc_mask;
uint32 os_specifies_revalidate;
@@ -7790,13 +7796,15 @@ HttpTransact::what_is_document_freshness
fresh_limit = min(NUM_SECONDS_IN_ONE_YEAR, fresh_limit);
}
- current_age =
- HttpTransactHeaders::calculate_document_age(s->request_sent_time,
- s->response_received_time,
- cached_obj_response, response_date, s->current.now);
-
- HTTP_DEBUG_ASSERT(current_age >= 0);
- current_age = min(NUM_SECONDS_IN_ONE_YEAR, current_age);
+ current_age = HttpTransactHeaders::calculate_document_age(s->request_sent_time,
+ s->response_received_time,
+ cached_obj_response, response_date, s->current.now);
+
+ // Overflow ?
+ if (current_age < 0)
+ current_age = NUM_SECONDS_IN_ONE_YEAR; // TODO: Should we make a new "max age" define?
+ else
+ current_age = min((time_t)NUM_SECONDS_IN_ONE_YEAR, current_age);
Debug("http_match", "[what_is_document_freshness] fresh_limit: %d current_age: %d", fresh_limit, current_age);
/////////////////////////////////////////////////////////
Modified: trafficserver/traffic/trunk/proxy/http2/HttpTransactCache.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http2/HttpTransactCache.cc?rev=1002726&r1=1002725&r2=1002726&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http2/HttpTransactCache.cc (original)
+++ trafficserver/traffic/trunk/proxy/http2/HttpTransactCache.cc Wed Sep 29 16:37:00 2010
@@ -222,6 +222,9 @@ HttpTransactCache::SelectFromAlternates(
current_age = HttpTransactHeaders::calculate_document_age(obj->request_sent_time_get(),
obj->response_received_time_get(),
cached_response, cached_response->get_date(), t_now);
+ // Overflow?
+ if (current_age < 0)
+ current_age = NUM_SECONDS_IN_ONE_YEAR; // TODO: Should we make a different define for "max cache age" ?
} else {
current_age = (time_t) 0;
}
Modified: trafficserver/traffic/trunk/proxy/http2/HttpTransactHeaders.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http2/HttpTransactHeaders.cc?rev=1002726&r1=1002725&r2=1002726&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http2/HttpTransactHeaders.cc (original)
+++ trafficserver/traffic/trunk/proxy/http2/HttpTransactHeaders.cc Wed Sep 29 16:37:00 2010
@@ -414,7 +414,7 @@ HttpTransactHeaders::calculate_document_
ink_time_t response_time,
HTTPHdr * base_response, ink_time_t base_response_date, ink_time_t now)
{
- ink_time_t age_value = 0;
+ ink_time_t age_value = base_response->get_age();
ink_time_t date_value = 0;
ink_time_t apparent_age = 0;
ink_time_t corrected_received_age = 0;
@@ -425,12 +425,10 @@ HttpTransactHeaders::calculate_document_
ink_time_t now_value = 0;
ink_time_t tmp_value = 0;
- age_value = base_response->get_age();
tmp_value = base_response_date;
date_value = (tmp_value > 0) ? tmp_value : 0;
-
// Deal with clock skew. Sigh.
//
// TODO solve this global clock problem
@@ -442,15 +440,17 @@ HttpTransactHeaders::calculate_document_
HTTP_DEBUG_ASSERT(now_value >= response_time);
if (date_value > 0) {
- apparent_age = max((ink_time_t) 0, (response_time - date_value));
+ apparent_age = max((time_t) 0, (response_time - date_value));
+ }
+ if (age_value < 0) {
+ current_age = -1; // Overflow from Age: header
+ } else {
+ corrected_received_age = max(apparent_age, age_value);
+ response_delay = response_time - request_time;
+ corrected_initial_age = corrected_received_age + response_delay;
+ resident_time = now_value - response_time;
+ current_age = corrected_initial_age + resident_time;
}
- corrected_received_age = max(apparent_age, age_value);
- response_delay = response_time - request_time;
- corrected_initial_age = corrected_received_age + response_delay;
- resident_time = now_value - response_time;
- current_age = corrected_initial_age + resident_time;
-
- HTTP_DEBUG_ASSERT(current_age >= 0);
if (diags->on()) {
DebugOn("http_age", "[calculate_document_age] age_value: %ld", age_value);
@@ -1055,9 +1055,9 @@ HttpTransactHeaders::insert_time_and_age
ink_time_t now, HTTPHdr * base, HTTPHdr * outgoing)
{
ink_time_t date = base->get_date();
- uint32 current_age = calculate_document_age(request_sent_time, response_received_time, base, date, now);
+ ink_time_t current_age = calculate_document_age(request_sent_time, response_received_time, base, date, now);
- outgoing->set_age(current_age);
+ outgoing->set_age(current_age); // set_age() deals with overflow properly, so pass it along
// FIX: should handle missing date when response is received, not here.
// See INKqa09852.