You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1999/01/28 10:29:42 UTC
[PATCH] etag continued
On 27 Jan 1999 coar@hyperreal.org wrote:
> + *) Fix checking of ETags and other opaque 'tokens' by adding
> + ap_find_opaque_token(). PR#2065, 3657 [Ken Coar]
> +
...
> --- 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_FAILED;
> return rstatus;
Unforunately, this is not quite everything.
In the if-match case, a strong etag comparison has to be performed.
That's pretty easy to handle -- just make sure etag[0] != 'W' && etag[1]
!= '/'.
In the if-none-match case, a strong etag comparison has to be performed
unless the method is GET or HEAD. In that case a weak comparison can be
used...
I think this patch finishes it. But I haven't tested it at all. Protocol
cops should check this.
Dean
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:27:50
@@ -401,10 +401,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 +434,25 @@
* 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
*/
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) {
+ if ((if_nonematch[0] == '*')
+ || ((etag != NULL)
+ && ((etag[0] == 'W' && etag[1] == '/'
+ && ap_find_opaque_token(r->pool, if_nonematch, etag + 2))
+ || ap_find_opaque_token(r->pool, if_nonematch, etag)))) {
+ return HTTP_NOT_MODIFIED;
+ }
+ }
+ else 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 HTTP_PRECONDITION_FAILED;
}
}
/* Else if a valid If-Modified-Since request-header field was given
Re: [PATCH] etag continued (take 2)
Posted by Dean Gaudet <dg...@arctic.org>.
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
Re: [PATCH] etag continued
Posted by Dean Gaudet <dg...@arctic.org>.
On Thu, 28 Jan 1999, Dean Gaudet wrote:
> In the if-none-match case, a strong etag comparison has to be performed
> unless the method is GET or HEAD. In that case a weak comparison can be
> used...
Oh yeah, it also needs to use strong if it's a range request... so my
patch is insufficient too. There could be other related bugs.
Dean