You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Roy T. Fielding" <fi...@kiwi.ics.uci.edu> on 1999/02/08 16:16:39 UTC
Re: cvs commit: apache-1.3/src/main gen_test_char.c http_protocol.c util.c
I'd like to reverse the following commit and replace it with the
equivalent using ap_get_list_item(). Does anyone object?
....Roy
In message <19...@hyperreal.org>,
coar@hyperreal.org writes:
>coar 99/01/27 04:16:04
>
> Modified: src CHANGES
> src/include ap_mmn.h httpd.h
> src/main gen_test_char.c http_protocol.c util.c
> Log:
> ETags aren't normal tokens by the RFC 2068 definition; they
> explicitly contain quoted strings and can include stuff *outside*
> the quotes as well ('W/' for a weak ETag). So add a new function
> that treats *everything* except ',' and ' ' (and not even those,
> if they're in a quoted string) as part of the token, and fix the
> ETag checks to use it.
>
> PR: 2065, 3657
>
> Revision Changes Path
> 1.1224 +3 -0 apache-1.3/src/CHANGES
>
> Index: CHANGES
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/CHANGES,v
> retrieving revision 1.1223
> retrieving revision 1.1224
> diff -u -r1.1223 -r1.1224
> --- CHANGES 1999/01/25 22:55:33 1.1223
> +++ CHANGES 1999/01/27 12:15:58 1.1224
> @@ -1,5 +1,8 @@
> Changes with Apache 1.3.5
>
> + *) Fix checking of ETags and other opaque 'tokens' by adding
> + ap_find_opaque_token(). PR#2065, 3657 [Ken Coar]
> +
> *) Add ability to handle DES or MD5 authentication passwords.
> [Ryan Bloom <rb...@Raleigh.IBM.Com>]
>
>
>
>
> 1.23 +3 -1 apache-1.3/src/include/ap_mmn.h
>
> Index: ap_mmn.h
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/include/ap_mmn.h,v
> retrieving revision 1.22
> retrieving revision 1.23
> diff -u -r1.22 -r1.23
> --- ap_mmn.h 1999/01/09 00:21:33 1.22
> +++ ap_mmn.h 1999/01/27 12:16:00 1.23
> @@ -203,6 +203,8 @@
> * scan_script_header -> ap_scan_script_header_err
> * - reordered entries in request_rec that were waitin
>g
> * for a non-binary-compatible release.
> + * 19990108-1 - add ap_find_opaque_token() for things like ETags
> + * (1.3.5-dev) which can contain opaque quoted strings
> */
>
> #define MODULE_MAGIC_COOKIE 0x41503133UL /* "AP13" */
> @@ -210,7 +212,7 @@
> #ifndef MODULE_MAGIC_NUMBER_MAJOR
> #define MODULE_MAGIC_NUMBER_MAJOR 19990108
> #endif
> -#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
> +#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */
> #define MODULE_MAGIC_NUMBER MODULE_MAGIC_NUMBER_MAJOR /* backward com
>pat */
>
> /* Useful for testing for features. */
>
>
>
> 1.265 +2 -0 apache-1.3/src/include/httpd.h
>
> Index: httpd.h
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/include/httpd.h,v
> retrieving revision 1.264
> retrieving revision 1.265
> diff -u -r1.264 -r1.265
> --- httpd.h 1999/01/10 07:48:59 1.264
> +++ httpd.h 1999/01/27 12:16:00 1.265
> @@ -936,6 +936,8 @@
>
> API_EXPORT(char *) ap_get_token(pool *p, const char **accept_line, int acce
>pt_white);
> API_EXPORT(int) ap_find_token(pool *p, const char *line, const char *tok);
> +API_EXPORT(int) ap_find_opaque_token(pool *p, const char *line,
> + const char *tok);
> API_EXPORT(int) ap_find_last_token(pool *p, const char *line, const char *t
>ok);
>
> API_EXPORT(int) ap_is_url(const char *u);
>
>
>
> 1.5 +9 -1 apache-1.3/src/main/gen_test_char.c
>
> Index: gen_test_char.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/gen_test_char.c,v
> retrieving revision 1.4
> retrieving revision 1.5
> diff -u -r1.4 -r1.5
> --- gen_test_char.c 1998/07/08 17:47:05 1.4
> +++ gen_test_char.c 1999/01/27 12:16:01 1.5
> @@ -8,6 +8,7 @@
> #define T_ESCAPE_PATH_SEGMENT (0x02)
> #define T_OS_ESCAPE_PATH (0x04)
> #define T_HTTP_TOKEN_STOP (0x08)
> +#define T_HTTP_OPAQUETOKEN_STOP (0x10)
>
> int main(int argc, char *argv[])
> {
> @@ -20,13 +21,15 @@
> "#define T_ESCAPE_PATH_SEGMENT (%u)\n"
> "#define T_OS_ESCAPE_PATH (%u)\n"
> "#define T_HTTP_TOKEN_STOP (%u)\n"
> +"#define T_HTTP_OPAQUETOKEN_STOP (%u)\n"
> "\n"
> "static const unsigned char test_char_table[256] = {\n"
> " 0,",
> T_ESCAPE_SHELL_CMD,
> T_ESCAPE_PATH_SEGMENT,
> T_OS_ESCAPE_PATH,
> - T_HTTP_TOKEN_STOP);
> + T_HTTP_TOKEN_STOP,
> + T_HTTP_OPAQUETOKEN_STOP);
>
> /* we explicitly dealt with NUL above
> * in case some strchr() do bogosity with it */
> @@ -52,6 +55,11 @@
> /* these are the "tspecials" from RFC2068 */
> if (ap_iscntrl(c) || strchr(" \t()<>@,;:\\/[]?={}", c)) {
> flags |= T_HTTP_TOKEN_STOP;
> + }
> +
> + /* some tokens (like etags) are opaque strings; stop at the end */
> + if (ap_iscntrl(c) || strchr(" ,", c)) {
> + flags |= T_HTTP_OPAQUETOKEN_STOP;
> }
> printf("%u%c", flags, (c < 255) ? ',' : ' ');
>
>
>
>
> 1.254 +4 -2 apache-1.3/src/main/http_protocol.c
>
> Index: http_protocol.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/http_protocol.c,v
> retrieving revision 1.253
> retrieving revision 1.254
> diff -u -r1.253 -r1.254
> --- http_protocol.c 1999/01/08 17:54:41 1.253
+++ http_protocol.c 1999/01/27 12:16:01 1.254
> @@ -404,7 +404,8 @@
> */
> if ((if_match = ap_table_get(r->headers_in, "If-Match")) != NULL) {
> if ((etag == NULL) ||
> - ((if_match[0] != '*') && !ap_find_token(r->pool, if_match, etag
>))) {
> + ((if_match[0] != '*')
> + && !ap_find_opaque_token(r->pool, if_match, etag))) {
> return HTTP_PRECONDITION_FAILED;
> }
> }
> @@ -437,7 +438,8 @@
> int rstatus;
>
> if ((if_nonematch[0] == '*')
> - || ((etag != NULL) && ap_find_token(r->pool, if_nonematch, etag
>))) {
> + || ((etag != NULL)
> + && ap_find_opaque_token(r->pool, if_nonematch, etag))) {
> rstatus = (r->method_number == M_GET) ? HTTP_NOT_MODIFIED
> : HTTP_PRECONDITION_FAILE
>D;
> return rstatus;
>
>
>
> 1.147 +61 -0 apache-1.3/src/main/util.c
>
> Index: util.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/util.c,v
> retrieving revision 1.146
> retrieving revision 1.147
> diff -u -r1.146 -r1.147
> --- util.c 1999/01/21 19:17:09 1.146
> +++ util.c 1999/01/27 12:16:02 1.147
> @@ -1057,6 +1057,67 @@
> }
> }
>
> +/*
> + * Find opaque HTTP tokens, which have no internal semantics and end
> + * at the first unquoted ',' or ' '.
> + */
> +API_EXPORT(int) ap_find_opaque_token(pool *p, const char *line,
> + const char *tok)
> +{
> + const unsigned char *start_token;
> + const unsigned char *s;
> + char stop_quote = '\0';
> +
> + if (!line) {
> + return 0;
> + }
> +
> + s = (const unsigned char *)line;
> + for (;;) {
> + /*
> + * Find start of token, skip all stop characters, note NUL
> + * isn't a token stop, so we don't need to test for it
> + */
> + while (TEST_CHAR(*s, T_HTTP_OPAQUETOKEN_STOP)) {
> + ++s;
> + }
> + if (!*s) {
> + return 0;
> + }
> + start_token = s;
> + /*
> + * Find end of the token
+ */
> + while (*s) {
> + /*
> + * If we see the beginning of a quoted string ("foo" or <foo>),
> + * mark the end character so we know when to stop skipping.
> + */
> + if (!stop_quote && ((*s == '"') || (*s == '<'))) {
> + stop_quote = (*s == '"') ? '"' : '>';
> + }
> + else if (*s == stop_quote) {
> + stop_quote = '\0';
> + }
> + /*
> + * If we're not inside a quoted string, check to see if we're
> + * at the end of the token.
> + */
> + else if (!stop_quote && TEST_CHAR(*s, T_HTTP_OPAQUETOKEN_STOP)) {
> + break;
> + }
> + ++s;
> + }
> + if (!strncmp((const char *)start_token, (const char *)tok,
> + s - start_token)) {
> + return 1;
> + }
> + if (!*s) {
> + return 0;
> + }
> + }
> +}
> +
>
> API_EXPORT(int) ap_find_last_token(pool *p, const char *line, const char *t
>ok)
> {
>
>
>
Re: cvs commit: apache-1.3/src/main gen_test_char.c http_protocol.c util.c
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Rodent of Unusual Size wrote:
>
> Roy T. Fielding wrote:
> >
> > I'd like to reverse the following commit and replace it with the
> > equivalent using ap_get_list_item(). Does anyone object?
>
> You mean rename/rework the 'opaque token' stuff into a more
> generic 'get a comma-separated list item' function? If so, +1..
'Never mind.' I read my new mail out of order. +1.
--
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: cvs commit: apache-1.3/src/main gen_test_char.c http_protocol.c util.c
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Roy T. Fielding wrote:
>
> I'd like to reverse the following commit and replace it with the
> equivalent using ap_get_list_item(). Does anyone object?
You mean rename/rework the 'opaque token' stuff into a more
generic 'get a comma-separated list item' function? If so, +1..
--
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: cvs commit: apache-1.3/src/main gen_test_char.c http_protocol.c util.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 8 Feb 1999, Roy T. Fielding wrote:
> I'd like to reverse the following commit and replace it with the
> equivalent using ap_get_list_item(). Does anyone object?
Could you also take into account my untested patch which relies on Ken's
change? We don't properly do strong and weak comparisons at the moment.
Dean
Date: Thu, 28 Jan 1999 01:58:37 -0800 (PST)
From: Dean Gaudet <dg...@arctic.org>
To: new-httpd@apache.org
Subject: Re: [PATCH] etag continued (take 2)
I love untested patches. Here's another. (I'm hoping I'll disgust one of
the protocol cops enough that they'll test it ;)
Ken's new function was only part of the fix. We still needed to enforce
strong comparisons in these cases:
- if-match is always strong
- if-none-match is strong unless the request is GET/HEAD and not a range
request
And also:
- if-range is always a strong comparison
- entity tags are case sensitive... if-range test was insensitive
Dean
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1225
diff -u -r1.1225 CHANGES
--- CHANGES 1999/01/28 07:15:16 1.1225
+++ CHANGES 1999/01/28 09:55:04
@@ -5,8 +5,11 @@
the NDBM library looks like Berkeley-DB based.
[Ralf S. Engelschall] PR#3773
- *) Fix checking of ETags and other opaque 'tokens' by adding
- ap_find_opaque_token(). PR#2065, 3657 [Ken Coar]
+ *) Entity tag comparisons for If-Match and If-None-Match were not being
+ performed correctly -- weak tags would cause false positives. Also,
+ strong comparison wasn't properly enforced in all cases. Added
+ new function ap_find_opaque_token().
+ [Ken Coar, Dean Gaudet] PR#2065, 3657
*) Add ability to handle DES or MD5 authentication passwords.
[Ryan Bloom <rb...@Raleigh.IBM.Com>]
Index: main/http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/http_protocol.c,v
retrieving revision 1.254
diff -u -r1.254 http_protocol.c
--- http_protocol.c 1999/01/27 12:16:01 1.254
+++ http_protocol.c 1999/01/28 09:55:05
@@ -148,8 +148,9 @@
if ((if_range = ap_table_get(r->headers_in, "If-Range"))) {
if (if_range[0] == '"') {
- if (!(match = ap_table_get(r->headers_out, "Etag")) ||
- (strcasecmp(if_range, match) != 0))
+ if (!(match = ap_table_get(r->headers_out, "Etag"))
+ || (match[0] == 'W' && match[1] == '/')
+ || (strcmp(if_range, match) != 0))
return 0;
}
else if (!(match = ap_table_get(r->headers_out, "Last-Modified")) ||
@@ -401,10 +402,12 @@
* AND if our ETag does not match any of the entity tags in that field
* AND the field value is not "*" (meaning match anything), then
* respond with a status of 412 (Precondition Failed).
+ * must use strong comparison
*/
if ((if_match = ap_table_get(r->headers_in, "If-Match")) != NULL) {
- if ((etag == NULL) ||
- ((if_match[0] != '*')
+ if ((etag == NULL)
+ || (etag[0] == 'W' && etag[1] == '/')
+ || ((if_match[0] != '*')
&& !ap_find_opaque_token(r->pool, if_match, etag))) {
return HTTP_PRECONDITION_FAILED;
}
@@ -432,17 +435,30 @@
* respond with a 304 (Not Modified) response.
* For all other request methods, the server MUST
* respond with a status of 412 (Precondition Failed).
+ * GET or HEAD allow weak comparison, all other methods require
+ * strong comparison. We can only use weak if it's not a range
+ * request.
*/
if_nonematch = ap_table_get(r->headers_in, "If-None-Match");
if (if_nonematch != NULL) {
- int rstatus;
-
- if ((if_nonematch[0] == '*')
+ if (r->method_number == M_GET
+ && !ap_table_get(r->headers_in, "Range")) {
+ /* we can use a weak comparison */
+ if (if_nonematch[0] == '*'
+ || (etag
+ && ((etag[0] == 'W' && etag[1] == '/' && etag[2]
+ && ap_find_opaque_token(r->pool, if_nonematch, etag + 2))
+ || ap_find_opaque_token(r->pool, if_nonematch, etag)))) {
+ return HTTP_NOT_MODIFIED;
+ }
+ }
+ /* must use a strong comparison */
+ if ((if_nonematch[0] == '*')
|| ((etag != NULL)
+ && (etag[0] != 'W' || etag[1] != '/')
&& ap_find_opaque_token(r->pool, if_nonematch, etag))) {
- rstatus = (r->method_number == M_GET) ? HTTP_NOT_MODIFIED
- : HTTP_PRECONDITION_FAILED;
- return rstatus;
+ return (r->method_number == M_GET)
+ ? HTTP_NOT_MODIFIED : HTTP_PRECONDITION_FAILED;
}
}
/* Else if a valid If-Modified-Since request-header field was given