Conversation
…entError, isServerError, getErrorMessage, debugInfo, etc.) Agent-Logs-Url: https://github.com/voku/httpful/sessions/f8fae34d-40e5-4db4-bb58-f13540c93ff2 Co-authored-by: voku <264695+voku@users.noreply.github.com>
…nfo() Agent-Logs-Url: https://github.com/voku/httpful/sessions/58761e7d-24bc-4816-a55c-38038269af56 Co-authored-by: voku <264695+voku@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces several helper methods to the Response class for identifying HTTP status ranges and generating human-readable error messages. It also adds a debugInfo method to provide a comprehensive summary of both the request and response for debugging purposes, along with corresponding unit tests. Feedback was provided regarding the potential exposure of sensitive information in the debugInfo output; specifically, headers such as Authorization, Cookie, and Set-Cookie should be redacted to prevent credential leakage in logs.
| foreach ($this->request->getHeaders() as $name => $values) { | ||
| $lines[] = $name . ': ' . \implode(', ', (array) $values); | ||
| } |
There was a problem hiding this comment.
The debugInfo() method currently leaks sensitive credentials by including raw header values for Authorization, Cookie, and Proxy-Authorization. Although the pull request description shows these being masked (e.g., Authorization: ******), the implementation does not actually perform redaction. For security reasons, sensitive headers should be redacted in debug output to prevent accidental exposure in logs.
foreach ($this->request->getHeaders() as $name => $values) {
$headerValue = \implode(', ', (array) $values);
if (\in_array(\strtolower($name), ['authorization', 'cookie', 'proxy-authorization'], true)) {
$headerValue = '******';
}
$lines[] = $name . ': ' . $headerValue;
}| foreach ($this->headers->toArray() as $name => $values) { | ||
| $lines[] = $name . ': ' . \implode(', ', (array) $values); | ||
| } |
There was a problem hiding this comment.
Response headers such as Set-Cookie can contain sensitive session information and should be redacted in the debug output to prevent leakage in logs.
foreach ($this->headers->toArray() as $name => $values) {
$headerValue = \implode(', ', (array) $values);
if (\strtolower($name) === 'set-cookie') {
$headerValue = '******';
}
$lines[] = $name . ': ' . $headerValue;
}
Response::hasErrors()was the only introspection tool, forcing callers to manually inspect raw status codes and craft their own error messages.debugInfo()also lacked request context, making it useless for diagnosing why a call failed.New API on
ResponseStatus-range predicates
isInformational()— 1xxisSuccess()— 2xxisRedirect()— 3xxisClientError()— 4xxisServerError()— 5xxgetErrorMessage(): stringHuman-readable explanation for the current status. Common codes (401, 403, 404, 422, 429, 500–504, …) get specific copy; other codes fall back to a range-level description.
debugInfo(): stringStructured multi-line dump now includes the full request context followed by the response:
The request section is omitted when no request is attached to the response. Request body is included when non-empty (POST/PUT payloads).
Usage
This change is