[GitHub] [tomcat] markt-asf commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
markt-asf commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672826428 After looking at b26ad77 I have reverted the addition of `useWeakComparisonWithIfMatch` and switched If-Match to strong comparison. I've also fixed the regression you identified. With these changes the If-Match and If-None-Match code has diverged a little further. I thin kthe case for refactoring is rather borderline at the moment but take a look at the new code and see what you think. 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] markt-asf commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
markt-asf commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672805585 Let me go an look at `b26ad77` in more detail. If the switch to weak comparison was introduced there then the case for fixing that and dropping `useWeakComparisonWithIfMatch` entirely is much stronger. 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] markt-asf commented on pull request #337: Bug fix: If-None-Match: 304 response should return a real resource ETag
markt-asf commented on pull request #337: URL: https://github.com/apache/tomcat/pull/337#issuecomment-672771845 In reviewing this PR I've noticed another issue. The ETag obtained from the resource is unconditionally made strong before the comparison but that conversion should depend on `useWeakComparisonWithIfMatch`. I'm not convinced of the case for aligning the two methods and fixing this new issue looks like it will weaken that case. The test case change looks good. I'll add that, add a test for the new issue and then look at a fix for both issues. 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
