[GitHub] [tomcat] stokito commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
stokito commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672871214 I reviewed and it looks fine and simpler. I just want to note here for history that according RFC 7232 sec 2.3.2 if resource's real etag is weak than strong comparison always failed: ETag 1 | ETag 2 | Strong Comparison | Weak Comparison -- | -- | -- | -- W/"1" | W/"1" | no match | match W/"1" | W/"2" | no match | no match W/"1" | "1" | no match | match "1" | "1" | match | match That's why in my version I added the note: // BZ 64265: By default Tomcat generates weak etag for resource. // But If-Match uses strong matching that expects a strong resource etag. // So we strip any leading "W/" and then compare using equals String resourceEtag = weakEtagToStrong(eTag); I.e. that code is a workaround for fact that Tomcat generates weak ETag for static resource while should generate strong (that was discussed in #324). Now when client will send the received weak etag it will always get 412. And this is correct according spec but may be unexpected for a client. But I'm pretty sure that nobody uses the `If-Match` anyway: a browser never sends it. I think that we should keep this version and instead in some feature releases switch Tomcat to generate strong etags by default. But this is another story. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] stokito commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
stokito commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672785490 I added a commit that extracts common matching logic This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] stokito commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
stokito commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672777683 yes, you are right and I wanted to discuss it latter. The PR fixes only one specific problem + small refactoring. The problem that I see is that the new `useWeakComparisonWithIfMatch` flag is not complaint with RFC because it always should be strict. An origin server MUST use the strong comparison function when comparing entity-tags for If-Match (Section 2.3.2), since the client intends this precondition to prevent the method from being applied if there have been any changes to the representation data. In fact as far I understood the weak comparison for `If-Match` was implemented by mistake in scope of b26ad77f659de34d49dda5a248566a38383cec0e. And this happened recently. That means that in fact here we making the bug as a feature. This may have sense if there is a lot of users who used this behavior but in fact we definitely haven't any of them. Did I miss something? Can we simplify logic and and inline useWeakComparisonWithIfMatch as a false? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
