traeak commented on PR #13858:
URL: https://github.com/apache/trafficserver/pull/13858#issuecomment-6279117378
## Summary
The diagnosis is correct: with
proxy.config.http.no_dns_just_forward_to_parent=1,
find_server_and_update_current_info() can take the DIRECT branch, flip
parent_result to FAIL and return HOST_NONE without setting
s->current.request_to. HandleCacheOpenReadHit tests the member (request_to ==
HOST_NONE && result == FAIL), so its guard is skipped and build_request() runs
with a null s->current.server. The added null check does stop the crash, but it
treats the symptom one level down. It also bypasses the existing serve-stale
logic, so a revalidation that could have returned the stale cached object now
always returns a 502. Fixing the inconsistent exit in
find_server_and_update_current_info() would let the existing guard handle this
case, including serving stale when allowed. The Format CI job is also failing
on the new lines.
## Critical Issues - Must Fix
1. **Fix is applied at the symptom, not the root cause, and skips the
serve-stale path**
- File: `src/proxy/http/HttpTransact.cc`
- The crash happens because the DIRECT + no_dns_forward_to_parent exit of
find_server_and_update_current_info() (around line 824) sets
parent_result.result = FAIL and returns HOST_NONE, but leaves
s->current.request_to unchanged. The PARENT FAIL exit just above it does set
request_to = HOST_NONE. Because of this, the guard at line 3023 in
HandleCacheOpenReadHit is not entered. With this PR, the new else branch
catches the null server and calls handle_parent_down(), so the client always
gets a 502. Before reaching it, though, the code has skipped the
is_stale_cache_response_returnable() check that the guard exists for. With the
root cause fixed, the existing guard serves the stale object when allowed and
calls handle_parent_down() otherwise. It also makes the two 'no upstream' exits
behave the same for every caller.
- Fix: In find_server_and_update_current_info(), set
s->current.request_to = ResolveInfo::HOST_NONE in the DIRECT +
no_dns_forward_to_parent branch before returning HOST_NONE, matching the FAIL
case. The new else branch can then be dropped, or kept as a defensive check
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
