Issue 2551175
Created on 2021-12-01 18:06 by rouilj, last changed 2021-12-02 00:54 by rouilj.
Messages | |||
---|---|---|---|
msg7392 | Author: [hidden] (rouilj) | Date: 2021-12-01 18:06 | |
When lookin at issue 2551173, it appears that the ETag header needs to be unique by including the content-encoding. From that ticket: ==== While I was looking into this, it turns out that the ETag must be different for different content-encodings. https://datatracker.ietf.org/doc/html/rfc7232#section-2.3.3. We do explicitly set the vary header to include accept-encoding, but apparently, that is not enough. I am not sure how that affects the use of ETag and intermediate caches. I suspect the easiest way to handle this is to take the ETag and append the content-encoding. So ETag: 584f82231079e349031bbb853747df1c-gzip Content-Encoding: gzip ETag: 584f82231079e349031bbb853747df1c-br Content-Encoding: br ETag: 584f82231079e349031bbb853747df1c-zstd Content-Encoding: zstd and then strip the suffix when comparing it server side. So as a client you don't care about the ETag value, it still remains opaque. But an intermediate cache will have three different copies that they can return depending on what the accept-header is. === No released version of roundup does content-encoding, so this isn't a huge issue but it should be fixed. I think it can be done in cgi/client.py::compress_encode where the header manipulation occurs at the end. The ETag header should have been added before this code is run. Something like: current_etag = self.additional_headers['ETag'] etag_end = current_etag.rindex('"') self.additional_headers['ETag'] = current_etag[:etag_end] + '-' + encoder \ current_etag[etag_end:] should work. Note that '-' doesn't show up in the output alphabet of hexdigest which is used to encode the etag. Tests will probably need to be done against the live server, but it may be possible to piggyback of a rest test or a compression test. |
|||
msg7393 | Author: [hidden] (rouilj) | Date: 2021-12-01 23:19 | |
Adding a little try/except/else block to the code above in the predicted spot works. However the @etag value returned as part of the json payload doesn't have the content-encoding suffix. I claim this is not an issue as: the @etag value in the payload is not used by caches, proxies or other HTTP level transit nodes using the un-suffixed etag in an HTTP conditional header will (doesn't at the moment) still work and allow the change. This is fine as the use of etag in this case still works to prevent the lost update problem. so I m not going to update the value of the payload's @etag value. |
|||
msg7394 | Author: [hidden] (rouilj) | Date: 2021-12-02 00:54 | |
Fixed in changeset: 6539:f8df7fed18f6 Tests, docs updated as well. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-01-09 02:33:33 | rouilj | link | issue2551173 superseder |
2021-12-02 00:54:32 | rouilj | set | status: open -> fixed resolution: fixed messages: + msg7394 |
2021-12-01 23:19:19 | rouilj | set | status: new -> open assignee: rouilj messages: + msg7393 nosy: + marcus.priesch |
2021-12-01 18:06:56 | rouilj | create |