Bug 64265

Summary:	ETag comparison does not properly implement RFC 7232, section 2.3.2
Product:	Tomcat 8	Reporter:	Michael Osipov <michaelo>
Component:	Catalina	Assignee:	Tomcat Developers Mailing List <dev>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P2
Version:	8.5.x-trunk
Target Milestone:	----
Hardware:	All
OS:	All

Description Michael Osipov 2020-03-25 14:01:23 UTC

The mentioned section provides two types of comparisons, strong and weak.

Here are the issues:
* It is not properly documented which comparison functions is applied by the DefaultServlet
* I believe that Tomcat implements either wrong.

Here is the code in question:
> while (!conditionSatisfied && commaTokenizer.hasMoreTokens()) {
> 	String currentToken = commaTokenizer.nextToken();
> 	if (currentToken.trim().equals(eTag))
> 		conditionSatisfied = true;
> }

This means that Tomcat performs char-by-char comparison. This already contradicts both functions (likely). A simple example:

$ curl "https://.../test/test.txt" -I
> HTTP/1.1 200
> Accept-Ranges: bytes
> ETag: W/"6-1585143822000"
> Last-Modified: Wed, 25 Mar 2020 13:43:42 GMT
> Content-Type: text/plain
> Content-Length: 6
> Date: Wed, 25 Mar 2020 13:55:50 GMT

Tomcat returns a weak etag, so try the weak function:
> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I
> HTTP/1.1 304
> ETag: W/"6-1585143822000"
> Date: Wed, 25 Mar 2020 13:58:01 GMT

This one should match with weak:
> $ curl "https://.../test/test.txt" -H 'If-None-Match: "6-1585143822000"' -I
> HTTP/1.1 200
> Accept-Ranges: bytes
> ETag: W/"6-1585143822000"
> Last-Modified: Wed, 25 Mar 2020 13:43:42 GMT
> Content-Type: text/plain
> Content-Length: 6
> Date: Wed, 25 Mar 2020 13:58:28 GMT

but it doesn't. It still returns 200.

If I try strong logically, the following should give me a 200:
> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I
> HTTP/1.1 304
> ETag: W/"6-1585143822000"
> Date: Wed, 25 Mar 2020 13:59:24 GMT

but it doesn't. It still returns 304.

Am I wrong here?

Comment 1 mgrigorov 2020-03-30 11:24:47 UTC

Hi Michael,

I see no difference between your 

> Tomcat returns a weak etag, so try the weak function:
>> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I

and

> If I try strong logically, the following should give me a 200:
>> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I

The commands are the same.

Comment 2 Michael Osipov 2020-03-30 13:05:09 UTC

(In reply to mgrigorov from comment #1)
> Hi Michael,
> 
> I see no difference between your 
> 
> > Tomcat returns a weak etag, so try the weak function:
> >> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I
> 
> and
> 
> > If I try strong logically, the following should give me a 200:
> >> $ curl "https://.../test/test.txt" -H 'If-None-Match: W/"6-1585143822000"' -I
> 
> The commands are the same.

Please read my comments also the defined comparison functions: strong and weak in the RFC.

Comment 3 mgrigorov 2020-03-30 13:58:04 UTC

> Please read my comments also the defined comparison functions: strong and weak in the RFC.

I just wanted to point out that the arguments of the two 'curl` commands are exactly the same. So receiving the same result is what I'd expect from the server, unless the resource is modified or deleted in the meantime.
Maybe you have a typo in the second curl command ?!

If this RFC says that two exactly the same requests should behave differently then I am not sure I want to read it.

Comment 4 Michael Osipov 2020-03-30 14:19:12 UTC

(In reply to mgrigorov from comment #3)
> > Please read my comments also the defined comparison functions: strong and weak in the RFC.
> 
> I just wanted to point out that the arguments of the two 'curl` commands are
> exactly the same. So receiving the same result is what I'd expect from the
> server, unless the resource is modified or deleted in the meantime.
> Maybe you have a typo in the second curl command ?!
> 
> If this RFC says that two exactly the same requests should behave
> differently then I am not sure I want to read it.

There is no typo and yes, both commands are the same. I have logically applied to comparsion functions. From a blackbox perspective, in either case Tomcat's implementation is wrong. Since we don't document which comparison we apply I have to guess, try.

Read the section and you'll understand.

Comment 5 Mark Thomas 2020-03-30 17:31:57 UTC

The DefaultServlet defers to the WebResources implementation to generate ETags.

The WebResource implementation provided by Tomcat is hard-coded to only provide weak ETags. It would be a fair amount of work for a custom implementation to override that (and none has ever asked us to make that easier).

Given the above, I think it makes sense for now for the DefaultServlet to perform the weak comparison. If the resource implementation changes, there could be an argument for the DefaultServlet to do something else.

I'll work on a patch (and test cases) for DefaultServlet.

Comment 6 Mark Thomas 2020-03-30 20:48:38 UTC

Fixed in:
- master for 10.0.0-M4 onwards
- 9.0.x for 9.0.34 onwards
- 8.5.x for 8.5.54 onwards
- 7.0.x for 7.0.104 onwards

Comment 7 Sergey Ponomarev 2020-07-30 13:46:19 UTC

We also should implement weak match for If-None-Match
I created PR with the fix
https://github.com/apache/tomcat/pull/325