shivampwm2020 opened a new pull request, #13852:
URL: https://github.com/apache/trafficserver/pull/13852
### Problem
`HttpTransact::handle_no_cache_operation_on_forward_server_response()` only
records
`VIA_SERVER_RESULT` / `VIA_PROXY_RESULT` for `200 OK` and `304 Not
Modified`. Responses
that reach the `HTTPStatus::PARTIAL_CONTENT` (206) or `default` branches are
passed
through to the client without setting either slot, so at
`proxy.config.http.insert_response_via_str = 2` the `Via` debug window is
identical for a
genuine origin response and for a locally synthesized error:
HTTP/1.1 206 Partial Content
Via: http/1.1 CDN (Up [cMs f ])
HTTP/1.1 500 Cannot find server. # DNS failure, origin never contacted
Via: http/1.1 CDN (Up [cMs f ])
Operators cannot distinguish "the origin answered 206" from "ATS synthesized
a 500"
without bumping verbosity to 3.
### Root cause
In the `switch` on the server response status, `HTTPStatus::OK` and
`HTTPStatus::NOT_MODIFIED` call `SET_VIA_STRING(...)`, while
`HTTPStatus::PARTIAL_CONTENT` and `default` do not:
```cpp
case HTTPStatus::PARTIAL_CONTENT:
ink_assert(s->cache_info.action == CacheAction_t::NO_ACTION);
s->next_action = StateMachineAction_t::SERVER_READ;
break;
default:
ink_assert(s->cache_info.action == CacheAction_t::NO_ACTION);
s->next_action = StateMachineAction_t::SERVER_READ;
break;
```
The cached counterpart,
`handle_cache_operation_on_forward_server_response()`, *does*
set both fields in its `default` branch, so the no-cache path is simply
inconsistent with
the cache path for the same class of response.
### Fix
Set `VIA_SERVER_RESULT = VIA_SERVER_SERVED` and `VIA_PROXY_RESULT =
VIA_PROXY_SERVED`
in the `PARTIAL_CONTENT` and `default` branches, matching the existing
200/304 branches in
the same function and the cached path. After the change the debug window is
`[cMsSf ]` for
a 206 or an origin 404, and is unchanged for a DNS-failure synthesized 500
(no server
connection was made, so the server slot correctly stays empty).
This is a 4-line change using markers that already appear verbatim in the
same function;
`via_string[]` layout, `VIA_*` indices and the `SET_VIA_STRING` macro are
untouched.
### Testing
- Confirmed the affected branches in `src/proxy/http/HttpTransact.cc` for
the `master`
branch: `PARTIAL_CONTENT` and `default` set neither slot before the change.
- The added lines are byte-for-byte identical to the existing calls in the
`OK` branch
(same function) and the cached path's `default` branch, so no new symbols
or headers
are introduced.
- Behaviour verified by inspection of `insert_via_header_in_response()`: the
`insert_response_via_string > 1` window covers `VIA_CACHE ..
VIA_CACHE_FILL_ACTION`,
which includes `VIA_SERVER_RESULT`.
Fixes #13118
Note: the linked issue was filed against `9.2.x`; the same branch is also
affected on
`master`. I opened this against `master` per `CONTRIBUTING.md` and am happy
to prepare a
`9.2.x` backport if the fix is accepted.
--
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]
