On 10/22/07, Jeff Trawick <[EMAIL PROTECTED]> wrote:
> like 2.0/2.2/trunk
attached is an updated patch for the boil-the-ocean flavor; at the
bottom is a tiny alternative
some ways to slice through the big patch:
1. my BIG 1.3 patch vs. the 2.0 commit
essentially same changes except for
s/apr_date_parse_http(date)/ap_parseHTTPdate(date)/
s/apr_rfc822_date(ndate, time)/ap_gm_timestr_822(p, t)/
and the fact that 2.0.x previously made a copy of the input date
string before parsing
The old 1.3.x version, without the logic to make a copy of the input,
temporarily modified the input date string, but it always restored the
original contents before returning.
2. use of ap_proxy_date_canon() in 1.3.x vs. 2.0.x
proxy_http.c callers in 2.0.x are the same as proxy_http.c callers in 1.3.x but
1.3.x also has callers in proxy_cache.c:
/* get the If-Modified-Since date of the request, if it exists */
c->ims = BAD_DATE;
datestr = ap_table_get(r->headers_in, "If-Modified-Since");
if (datestr != NULL) {
/* this may modify the value in the original table */
datestr = ap_proxy_date_canon(r->pool, datestr);
c->ims = ap_parseHTTPdate(datestr);
if (c->ims == BAD_DATE) /* bad or out of range date; remove it */
ap_table_unset(r->headers_in, "If-Modified-Since");
}
/* get the If-Unmodified-Since date of the request, if it exists */
c->ius = BAD_DATE;
datestr = ap_table_get(r->headers_in, "If-Unmodified-Since");
if (datestr != NULL) {
/* this may modify the value in the original table */
datestr = ap_proxy_date_canon(r->pool, datestr);
c->ius = ap_parseHTTPdate(datestr);
if (c->ius == BAD_DATE) /* bad or out of range date; remove it */
ap_table_unset(r->headers_in, "If-Unmodified-Since");
}
The comment "this may modify the value in the original table" is
perhaps alarming, but since ap_proxy_date_canon() restored the
original value, proxy_cache.c isn't relying on the modification as a
useful side-effect.
The call to ap_parseHTTPdate() right after ap_proxy_date_canon() is
more obviously wasteful now, since ap_parseHTTPdate() is the first
thing that ap_proxy_date_canon() calls, and the new string is
discarded.Thus the block for handling each date header can be
simplified to:
/* get the If-Modified-Since date of the request, if it exists and
is valid */
datestr = ap_table_get(r->headers_in, "If-Modified-Since");
if (datestr != NULL) {
c->ims = ap_parseHTTPdate(datestr);
if (c->ims == BAD_DATE) /* bad or out of range date; remove it */
ap_table_unset(r->headers_in, "If-Modified-Since");
}
else {
c->ims = BAD_DATE;
}
The proxy_cache.c code could be left alone with its extra parsing and
formatting, but as it deserves testing anyway it is best to test the
simpler path than the more complex one.
3. apr_date_parse_http(date) vs. ap_parseHTTPdate(date)
apr_date_parse_http supports an additional RFC 1123 variation with
only one digit for day of month ("6 Oct 2007" instead of "06 Oct
2007"). The old ap_proxy_date_canon() didn't support either, so it
doesn't matter.
4. apr_rfc822_date(ndate, time) vs. ap_gm_timestr_822(p, t)
same format created
5. other comments
ap_parseHTTPdate() ignores the name of the day but old
ap_proxy_date_canon() verified it. ap_parseHTTPdate() doesn't
actually need the day name to compute the proper time.
The new version has some extra math in ap_gm_timestr_822()'s call to
gmtime() , and calls ap_psprintf() instead of ap_palloc(p, 30) +
ap_snprintf().
--/--
OTOH, a couple of strlen() calls in the original code would be a more
pragmatic solution.
Index: src/modules/proxy/proxy_util.c
===================================================================
--- src/modules/proxy/proxy_util.c (revision 588101)
+++ src/modules/proxy/proxy_util.c (working copy)
@@ -282,7 +282,8 @@
*q = ',';
if (wk == 7)
return x; /* not a valid date */
- if (q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
+ if (strlen(q) != 24 ||
+ q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
q[17] != ':' || strcmp(&q[20], " GMT") != 0)
return x;
if (sscanf(q + 2, "%u-%3s-%u %u:%u:%u %3s", &mday, month, &year,
@@ -295,7 +296,8 @@
}
else {
/* check for acstime() date */
- if (x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
+ if (strlen(x) != 24 ||
+ x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
x[16] != ':' || x[19] != ' ' || x[24] != '\0')
return x;
if (sscanf(x, "%3s %3s %u %u:%u:%u %u", week, month, &mday, &hour,
I vote for the small patch, and canl also throw in a spell check for
that last comment above.
Index: src/modules/proxy/proxy_util.c
===================================================================
--- src/modules/proxy/proxy_util.c (revision 586541)
+++ src/modules/proxy/proxy_util.c (working copy)
@@ -256,9 +256,6 @@
return NULL;
}
-static const char *const lwday[7] =
-{"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};
-
/*
* If the date is a valid RFC 850 date or asctime() date, then it
* is converted to the RFC 1123 format, otherwise it is not modified.
@@ -267,61 +264,17 @@
* formatted, then it exits very quickly.
*/
const char *
- ap_proxy_date_canon(pool *p, const char *x)
+ ap_proxy_date_canon(pool *p, const char *date)
{
- int wk, mday, year, hour, min, sec, mon;
- char *q, month[4], zone[4], week[4];
+ time_t t;
- q = strchr(x, ',');
- /* check for RFC 850 date */
- if (q != NULL && q - x > 3 && q[1] == ' ') {
- *q = '\0';
- for (wk = 0; wk < 7; wk++)
- if (strcmp(x, lwday[wk]) == 0)
- break;
- *q = ',';
- if (wk == 7)
- return x; /* not a valid date */
- if (q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
- q[17] != ':' || strcmp(&q[20], " GMT") != 0)
- return x;
- if (sscanf(q + 2, "%u-%3s-%u %u:%u:%u %3s", &mday, month, &year,
- &hour, &min, &sec, zone) != 7)
- return x;
- if (year < 70)
- year += 2000;
- else
- year += 1900;
+ t = ap_parseHTTPdate(date);
+ if (t == BAD_DATE) {
+ return date;
}
- else {
-/* check for acstime() date */
- if (x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
- x[16] != ':' || x[19] != ' ' || x[24] != '\0')
- return x;
- if (sscanf(x, "%3s %3s %u %u:%u:%u %u", week, month, &mday, &hour,
- &min, &sec, &year) != 7)
- return x;
- for (wk = 0; wk < 7; wk++)
- if (strcmp(week, ap_day_snames[wk]) == 0)
- break;
- if (wk == 7)
- return x;
- }
-
-/* check date */
- for (mon = 0; mon < 12; mon++)
- if (strcmp(month, ap_month_snames[mon]) == 0)
- break;
- if (mon == 12)
- return x;
-
- q = ap_palloc(p, 30);
- ap_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", ap_day_snames[wk],
mday,
- ap_month_snames[mon], year, hour, min, sec);
- return q;
+ return ap_gm_timestr_822(p, t);
}
-
/*
* Reads headers from a buffer and returns an array of headers.
* Returns NULL on file error
Index: src/modules/proxy/proxy_cache.c
===================================================================
--- src/modules/proxy/proxy_cache.c (revision 586541)
+++ src/modules/proxy/proxy_cache.c (working copy)
@@ -929,27 +929,27 @@
c->hdrs = NULL;
c->xcache = NULL;
- /* get the If-Modified-Since date of the request, if it exists */
- c->ims = BAD_DATE;
+ /* get the If-Modified-Since date of the request, if it exists and is
valid */
datestr = ap_table_get(r->headers_in, "If-Modified-Since");
if (datestr != NULL) {
- /* this may modify the value in the original table */
- datestr = ap_proxy_date_canon(r->pool, datestr);
c->ims = ap_parseHTTPdate(datestr);
if (c->ims == BAD_DATE) /* bad or out of range date; remove it */
ap_table_unset(r->headers_in, "If-Modified-Since");
}
+ else {
+ c->ims = BAD_DATE;
+ }
-/* get the If-Unmodified-Since date of the request, if it exists */
- c->ius = BAD_DATE;
+ /* get the If-Unmodified-Since date of the request, if it exists */
datestr = ap_table_get(r->headers_in, "If-Unmodified-Since");
if (datestr != NULL) {
- /* this may modify the value in the original table */
- datestr = ap_proxy_date_canon(r->pool, datestr);
c->ius = ap_parseHTTPdate(datestr);
if (c->ius == BAD_DATE) /* bad or out of range date; remove it */
ap_table_unset(r->headers_in, "If-Unmodified-Since");
}
+ else {
+ c->ius = BAD_DATE;
+ }
/* get the If-Match of the request, if it exists */
c->im = ap_table_get(r->headers_in, "If-Match");
