[GitHub] [trafficserver] elsloo opened a new pull request, #9534: (backport) Fixes silent header duplication and early loop termination that occurs upon successful revalidation when duplicate headers are present

["elsloo (via GitHub)" <gi...@apache.org>]

elsloo opened a new pull request, #9534:
URL: https://github.com/apache/trafficserver/pull/9534

   - Initial attempt to backport the fix (#9527) for the duplicate header bug on revalidations to 9.2.x.
   - Inner loop's auto `spot2` iterator contains a member variable, `_block` that references the memory address of the response headers, not the cached headers
     - Method called on the `cached_headers` object uses a memory address, not a named header, and leads to all response headers being deleted from the duplicate header forward
     - Once response headers are deleted, the first duplicate header is written to the cached headers as a new and *additional* field which leads to duplication in the cached object's headers, then the current iteration ends
     - The iterator of the outer loop then attempts to increment forward one step via the overridden `++` operator on `spot`, and due to header slot checks via `MIMEField::is_live()` within `MIMEHdrImpl::iterator::step()`, `_slot` is incremented to `limit` and iteration of the outer loop halts, because all response headers from that point forward were deleted in the prior iteration
   - This change moves away from the address-based approach to delete the header and instead uses the header name
     - Using the header name instead of relying on header alignment also fixes a secondary issue that could arise if response header ordering does not match the cached object's header ordering
     - Using the header name is slightly less efficient due to having to call `find_header`, however, this is necessary to ensure the cached headers are correctly removed before the response headers are merged into the cached object
     - Using a slightly less efficient approach that occurs only on successful revalidations that also contain duplicate headers should be acceptable given the tradeoff is allowing duplication if response header ordering differs from the cached object
   - Due to the existing logic, if a cached header is *not* in the response, it will remain in the cached object's headers
   
   Thanks to Masakazu and Leif for providing the unit test and helping with the fix, respectively.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] bneradt commented on pull request #9534: (backport) Fixes silent header duplication and early loop termination that occurs upon successful revalidation when duplicate headers are present

["bneradt (via GitHub)" <gi...@apache.org>]

bneradt commented on PR #9534:
URL: https://github.com/apache/trafficserver/pull/9534#issuecomment-1495068345

   @elsloo : can you please re-merge this to resolve conflicts.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zwoop merged pull request #9534: (backport) Fixes silent header duplication and early loop termination that occurs upon successful revalidation when duplicate headers are present

["zwoop (via GitHub)" <gi...@apache.org>]

zwoop merged PR #9534:
URL: https://github.com/apache/trafficserver/pull/9534


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org