Repository navigation
Conversation
|
Hi @jpadilla, could you take a look at this when you have a moment? Happy to fix up anything that needs changing. Thanks! |
|
Hi @smoltafet! This change doesn't seem to parse the headers correctly. For example, it doesn't parse repeated field lines right and drops all except the last. I recommend checking out the relevant RFC: https://www.rfc-editor.org/rfc/rfc9110.html#name-field-lines-and-combined-fi. import io
from http.client import parse_headers
from urllib.response import addinfourl
from jwt.jwks_client import PyJWKClient
raw_headers = (
b"Content-Type: application/json\r\n"
b"Cache-Control: public\r\n"
b"Cache-Control: max-age=3600\r\n"
b"\r\n"
)
headers = parse_headers(io.BytesIO(raw_headers))
response = addinfourl(
io.BytesIO(b'{"keys": []}'),
headers,
"https://example.com/.well-known/jwks.json",
code=200,
)
print(response.headers)
print(PyJWKClient._extract_response_headers(response))The proposed _extract_response_headers drops the first I hope the example is clear. I might have of course missed something, so reach out if anything isn't clear! |
RFC 9110 section 5.2: a field name can appear on multiple field lines, and the combined value joins them with ", ". The dict comprehension in _extract_response_headers kept only the last line, so e.g. Cache-Control: public followed by Cache-Control: max-age=3600 lost the public directive. Group duplicate names (case-insensitively, keeping the first spelling) and join their values. Adds a regression test using the reporter's parse_headers example.
|
@Graph3x fixed, thanks for the clear example. Repeated header lines are now combined with ", " per RFC 9110 instead of keeping only the last one, so your example returns Cache-Control as "public, max-age=3600". Added a regression test with that exact case. Full suite passes locally (481 passed, 4 skipped). |
|
Hi! The new commit handles the exact example I posted, but not the underlying issue. Flattening the headers into a I'm not a maintainer, but given you have opened 40+ PRs across many repositories in the last 3 days, I doubt there's time for the back and forth this would need and it would likely take longer than writing this myself. Happy to put together an alternative PR if the maintainers would like one, or help with the review if they decide they prefer this design. Cheers! |
Graph3x pointed out the dict shape is the underlying problem, not just the combining: RFC 9110 section 5.3 says Set-Cookie must not be combined, and its values contain commas (Expires), so any dict either drops repeated lines or corrupts them.
last_response_headers now returns an http.client.HTTPMessage copy holding every field line verbatim, in order. Repeated fields are read with get_all(), e.g. get_all("Set-Cookie"). _extract_response_headers no longer combines anything. Tests updated to the new shape, plus a regression test with repeated Cache-Control and Set-Cookie lines.
|
@Graph3x you're right, the dict itself was the problem. Reworked it so last_response_headers returns an HTTPMessage copy with every field line kept separate, repeated Set-Cookie included, readable with get_all. Also my girl left me so I got the extra time now to contribute to open-source (she cheated on me). |
Fixes #1166. (This is only the header-exposure half; it does not implement the
max-agecaching behaviour discussed in #1165.)Problem
PyJWKClientmade no response headers available after a JWKS fetch, so an application implementing CDN-bypass on kid-miss could not align its logic with the provider's stated intent: a parsedmax-agealone loses directives such asstale-while-revalidate=86400, and the fullCache-Controlheader was simply unreachable.Change
Least-invasive API shape: a read-only attribute on the client, no signature or return-type changes.
PyJWKClient.fetch_data()captures the headers of every successful response (best-effort extraction from the urllib response'sHTTPMessage, degrading to an empty message for responses without a header mapping) and stores them on the client.PyJWKClient.last_response_headers -> http.client.HTTPMessagereturns a copy of the full, unparsed header set from the most recent successful fetch — e.g.Cache-Control: public, max-age=600, stale-while-revalidate=86400is available verbatim. It returns the message type rather than a plain dict because a field can appear on multiple lines and some must not be combined: RFC 9110 §5.3 singles outSet-Cookie, whose values contain commas (e.g. inExpires). Every field line is preserved verbatim, in order, and repeated fields are read withget_all(name). (Thanks to @Graph3x for catching that the earlier dict shape dropped/corrupted repeated lines.)Tests
9 new tests in
tests/test_jwks_client.pywith mockedurlopenresponses carrying a realhttp.client.HTTPMessage: fullCache-Controlexposure (incl.stale-while-revalidate), preservation of repeated field lines (repeatedCache-ControlandSet-Cookieread back separately viaget_all()), availability afterget_signing_keys(), cache-hit preservation, refresh replacement, failed-fetch preservation, header-less responses, copy semantics, and the pre-fetch empty state.pytest tests/test_jwks_client.py— 57 passed. Full suite — 481 passed, 4 skipped (pristine baseline: 472 passed, 4 skipped; zero new failures).