diff --git a/NEWS b/NEWS index 51cf5eee..ea7c00b0 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,7 @@ NEWS * escape all strings for logging (fixes #2646 log file injection, reported by Jaanus Kääp) * fix hex escape in accesslog (fixes #2559) * show extforward re-run warning only with debug.log-request-handling (fixes #2561) + * parse If-None-Match for ETag validation (fixes #2578) - 1.4.35 - 2014-03-12 * [network/ssl] fix build error if TLSEXT is disabled diff --git a/src/etag.c b/src/etag.c index f8fb6094..adc8a04c 100644 --- a/src/etag.c +++ b/src/etag.c @@ -9,8 +9,139 @@ #include -int etag_is_equal(buffer *etag, const char *matches) { - if (etag && !buffer_string_is_empty(etag) && 0 == strcmp(etag->ptr, matches)) return 1; +int etag_is_equal(buffer *etag, const char *line, int weak_ok) { + enum { + START = 0, + CHECK, + CHECK_QUOTED, + SKIP, + SKIP_QUOTED, + TAIL + } state = START; + + const char *current; + const char *tok_start = etag->ptr; + const char *tok = NULL; + int matched; + + if ('*' == line[0] && '\0' == line[1]) { + return 1; + } + + if (!etag || buffer_string_is_empty(etag)) return 0; + + if ('W' == tok_start[0]) { + if (!weak_ok || '/' != tok_start[1]) return 0; /* bad etag */ + tok_start = tok_start + 2; + } + + if ('"' != tok_start[0]) return 0; /* bad etag */ + /* we start comparing after the first '"' */ + ++tok_start; + + for (current = line; *current; ++current) { + switch (state) { + case START: + /* wait for etag to start; ignore whitespace and ',' */ + switch (*current) { + case 'W': + /* weak etag always starts with 'W/"' */ + if ('/' != *++current) return 0; /* bad etag list */ + if ('"' != *++current) return 0; /* bad etag list */ + if (!weak_ok) { + state = SKIP; + } else { + state = CHECK; + tok = tok_start; + } + break; + case '"': + /* strong etag starts with '"' */ + state = CHECK; + tok = tok_start; + break; + case ' ': + case ',': + case '\t': + case '\r': + case '\n': + break; + default: + return 0; /* bad etag list */ + } + break; + case CHECK: + /* compare etags (after the beginning '"') + * quoted-pairs must match too (i.e. quoted in both strings): + * > (RFC 2616:) both validators MUST be identical in every way + */ + matched = *tok && *tok == *current; + ++tok; + switch (*current) { + case '\\': + state = matched ? CHECK_QUOTED : SKIP_QUOTED; + break; + case '"': + if (*tok) { + /* bad etag - string should end after '"' */ + return 0; + } + if (matched) { + /* matching etag: strings were equal */ + return 1; + } + + state = TAIL; + break; + default: + if (!matched) { + /* strings not matching, skip remainder of etag */ + state = SKIP; + } + break; + } + break; + case CHECK_QUOTED: + if (!*tok || *tok != *current) { + /* strings not matching, skip remainder of etag */ + state = SKIP; + break; + } + ++tok; + state = CHECK; + break; + case SKIP: + /* wait for final (not quoted) '"' */ + switch (*current) { + case '\\': + state = SKIP_QUOTED; + break; + case '"': + state = TAIL; + break; + } + break; + case SKIP_QUOTED: + state = SKIP; + break; + case TAIL: + /* search for ',', ignore white space */ + switch (*current) { + case ',': + state = START; + break; + case ' ': + case '\t': + case '\r': + case '\n': + break; + default: + return 0; /* bad etag list */ + } + break; + } + } + /* no matching etag found */ return 0; } diff --git a/src/etag.h b/src/etag.h index 97cb0631..41ebc99b 100644 --- a/src/etag.h +++ b/src/etag.h @@ -9,7 +9,7 @@ typedef enum { ETAG_USE_INODE = 1, ETAG_USE_MTIME = 2, ETAG_USE_SIZE = 4 } etag_flags_t; -int etag_is_equal(buffer *etag, const char *matches); +int etag_is_equal(buffer *etag, const char *matches, int weak_ok); int etag_create(buffer *etag, struct stat *st, etag_flags_t flags); int etag_mutate(buffer *mut, buffer *etag); diff --git a/src/http-header-glue.c b/src/http-header-glue.c index 752d91e8..4f970fe4 100644 --- a/src/http-header-glue.c +++ b/src/http-header-glue.c @@ -244,7 +244,11 @@ buffer * strftime_cache_get(server *srv, time_t last_mod) { int http_response_handle_cachable(server *srv, connection *con, buffer *mtime) { + int head_or_get = + ( HTTP_METHOD_GET == con->request.http_method + || HTTP_METHOD_HEAD == con->request.http_method); UNUSED(srv); + /* * 14.26 If-None-Match * [...] @@ -255,12 +259,12 @@ int http_response_handle_cachable(server *srv, connection *con, buffer *mtime) { * return a 304 (Not Modified) response. */ - /* last-modified handling */ if (con->request.http_if_none_match) { - if (etag_is_equal(con->physical.etag, con->request.http_if_none_match)) { - if (con->request.http_method == HTTP_METHOD_GET || - con->request.http_method == HTTP_METHOD_HEAD) { - + /* use strong etag checking for now: weak comparison must not be used + * for ranged requests + */ + if (etag_is_equal(con->physical.etag, con->request.http_if_none_match, 0)) { + if (head_or_get) { con->http_status = 304; return HANDLER_FINISHED; } else { @@ -269,9 +273,8 @@ int http_response_handle_cachable(server *srv, connection *con, buffer *mtime) { return HANDLER_FINISHED; } } - } else if (con->request.http_if_modified_since && - (con->request.http_method == HTTP_METHOD_GET || - con->request.http_method == HTTP_METHOD_HEAD)) { + } else if (con->request.http_if_modified_since && head_or_get) { + /* last-modified handling */ size_t used_len; char *semicolon; diff --git a/tests/cachable.t b/tests/cachable.t index 381f44e0..cd47eb2d 100755 --- a/tests/cachable.t +++ b/tests/cachable.t @@ -8,7 +8,7 @@ BEGIN { use strict; use IO::Socket; -use Test::More tests => 13; +use Test::More tests => 25; use LightyTest; my $tf = LightyTest->new(); @@ -117,5 +117,105 @@ EOF $t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 200 } ]; ok($tf->handle_http($t) == 0, 'Conditional GET - ETag + disabled etags on server side'); +############### + +ok($etag =~ /^\"(.*)\"$/, "The server must quote ETags"); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 200 } ]; +ok($tf->handle_http($t) == 0, 'The client must send a quoted ETag'); + +$etag =~ /^(\".*)\"$/; +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 200 } ]; +ok($tf->handle_http($t) == 0, 'The ETag must be surrounded by quotes'); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; +ok($tf->handle_http($t) == 0, 'An unquoted star matches any ETag'); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 200 } ]; +ok($tf->handle_http($t) == 0, 'A quoted star is just a regular ETag'); + +TODO: { + local $TODO = "weak etags not allowed yet"; + $t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; + ok($tf->handle_http($t) == 0, 'A weak etag matches like a regular ETag for HEAD and GET'); +} + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 206, 'HTTP-Content' => '<' } ]; +ok($tf->handle_http($t) == 0, 'A weak etag does not match for ranged requests'); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 200 } ]; +ok($tf->handle_http($t) == 0, 'However, a weak ETag is not *'); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; +ok($tf->handle_http($t) == 0, 'Client sent a list of ETags, the second matches'); + +TODO: { + local $TODO = "weak etags not allowed yet"; + $t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; + ok($tf->handle_http($t) == 0, 'The second provided ETag matches weakly'); +} + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; +ok($tf->handle_http($t) == 0, 'Broken client did get around to sending good data'); + +$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 304 } ]; +ok($tf->handle_http($t) == 0, 'Bad syntax *after* a matching ETag doesn\'t matter'); + ok($tf->stop_proc == 0, "Stopping lighttpd");