|Summary:||Fix ResourceAttributes ETag handling|
|Product:||Tomcat 6||Reporter:||Chris Hubick <chris>|
|Component:||Catalina||Assignee:||Tomcat Developers Mailing List <dev>|
|Attachments:||Fix ResourceAttributes ETag handling|
Description Chris Hubick 2008-09-27 15:07:04 UTC
Created attachment 22642 [details] Fix ResourceAttributes ETag handling The ResourceAttributes class and it's clients have a few problems WRT ETag handling: - ResourceAttributes only has a "getetag" attribute (apparently for WebDav?), and doesn't expose this value through an HTTP header compatible ALTERNATE_ETAG attribute like it does it's other attributes. - ResourceAttributes doesn't expose it's ETag through the getIDs(), getAll(), or get(String) methods, meaning FileDirContext.FileResourceAttributes won't return the value to DirContextURLConnection clients along with the other attributes. - The ResourceAttributes API is confusing, in that setETag(String) sets the *strong* etag, but getETag() returns the *weak* etag. - ResourceAttributes.getETag(boolean) is broken in that it sets the 'result' variable if there is an ETAG attribute value, but then immediately proceeds to overwrite that result value with the 'strongETag' or 'weakETag' value. - The only user of ResourceAttributes.getETag(boolean) is DefaultServlet.getETag(ResourceAttributes), which calls both ResourceAttributes.getETag() and ResourceAttributes.getETag(boolean) in succession - this method can be totally removed (making access more consistent with other attributes) by having just the simpler ResourceAttributes.getETag() return a strong tag if there is one, else fallback to returning the weak one. - The DefaultServlet.getETag(ResourceAttributes) method has a redundant and dead-code path to generate a weak etag using the content length and last modified values - which will have already been done and returned by ResourceAttributes.getETag(boolean). The attached patch against trunk should fix all of this. FYI: This is a continuation of the DirContextURLConnection problems I fixed in bug 44611.
Comment 1 Chris Hubick 2008-09-27 15:55:00 UTC
Sorry, this may be a dup of bug 45735, but provides more extensive cleanups.
Comment 2 Mark Thomas 2008-09-29 06:29:53 UTC
I have applied the patch to trunk with a minor modification (setETag was not renamed). I will propose it for 6.0.x. Many thanks.
Comment 3 Chris Hubick 2008-09-29 09:11:05 UTC
Sounds great Mark, I renamed that before I realized getETag() could replace getETag(boolean) and return either the weak or strong value. With the single generic getter, having a single generic setter makes more sense. Anyhow, the quick response makes it a pleasure to contribute, so thanks for helping get the fix in :)
Comment 4 Mark Thomas 2008-10-27 04:52:33 UTC
This has been applied to 6.0.x (with getETag(boolean) marked as deprecated) and will be included in 6.0.19 onwards. Thanks again for the patch.