Repository navigation
FIX: Include endpoint and method in HTTPXAPITarget identifier - #3374
Roman Lutz (romanlutz) merged 10 commits into
Conversation
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
Roman Lutz (romanlutz)
left a comment
There was a problem hiding this comment.
Please address the credential exposure and the remaining identifier collision described below before merging.
|
Roman Lutz (@romanlutz) thanks for checking this PR I will update it as soon as possible |
…entifier Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
|
Thanks for the thorough review Roman Lutz (@romanlutz). I have updated the implementation to address both points. The target now resolves http_url against any configured base_url via httpx.URL(base_url).join(http_url) before building the identifier and origin. This ensures relative paths pointing to different hosts or base paths resolve their destination origin and avoid hash collisions. For credential handling, the identifier endpoint now strips userinfo (username and password) and filters out sensitive query parameters like keys, tokens, secrets, signatures, and auth parameters. The full raw URL is preserved on the target for actual outbound requests. I also added unit tests covering credential redaction from the identifier dump, base_url path resolution across distinct hosts, and credential stripping when credentials exist in base_url. |
… with httpx URL semantics Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
|
Roman Lutz (@romanlutz) both follow-ups are addressed in f7312f8. P1 — remaining credential exposure:
P2 — URL resolution semantics:
The full test file passes (22 tests) and Ruff check/format are clean. Requesting re-review — thanks again for the thorough validation. |
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
|
Roman Lutz (@romanlutz) fixed in 6925784. P2 — substring false positives:
Full test file passes (23 tests), Ruff check/format clean. Requesting re-review. |
|
Summary of the review findings and the discussions that are still open:
These findings were reproduced against |
|
Re-reviewed One P1 finding is still open: camelCase credential names such as Please address that remaining credential-exposure issue before merging. |
Description
Fixes #3373
HTTPXAPITarget previously delegated raw request setup to HTTPTarget with an empty string, causing the target endpoint to remain an empty string. It also inherited HTTPTarget._build_identifier without overriding it, leaving prompt_regex_string and use_tls while omitting method and http2. This change sets the endpoint and destination origin to the configured http_url and implements _build_identifier to capture method, follow_redirects, callback_function, and http2 so that distinct API targets produce distinct identifiers.
Tests and Documentation
Added unit tests in test_http_api_target.py verifying that HTTPXAPITarget populates its endpoint and method parameters properly, and that distinct endpoints and methods produce distinct identifier hashes.
No documentation change needed, so I did not run JupyText.