Repository navigation
Replace HTTPoison with Req in the ACMEv2 client - #8
Merged
Merged
Conversation
All HTTP calls in the ACMEv2 client are plain GET/POST, so Req (Finch-based)
is a drop-in for HTTPoison:
- request/5 uses Req.request! with decode_body: false (callers Jason.decode
the raw JSON/JOSE body themselves), retry: false (the function already
loops on failure), and redirect: false (HTTPoison parity; an ACME POST
binds the request URL into the JWS protected header, so a transparently
re-POSTed redirect would fail signature validation anyway).
- Replay-Nonce / Location response headers are read via
Req.Response.get_header/2 (Req lowercases response header names).
- status_code -> status and %HTTPoison.Response{} -> %Req.Response{} at all
call sites; mixed string/atom request-header keys normalized for Req.
- Fixes a latent retry bug: the recursive retry call passed (uri, method)
into a (method, uri) head, so every retried request was malformed.
- Dep swap: httpoison ~> 2.0 -> req ~> 0.5, dropping the hackney tree.
Verified: mix test, credo, format; smoke test against Let's Encrypt staging
(directory + newNonce + Replay-Nonce extraction) with the exact Req options.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
Owner
|
I do prefer Req as well |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
6A24
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All of the ACMEv2 client's HTTP calls are plain GET/POST, so Req (Finch-based) is a drop-in for HTTPoison. This drops the httpoison → hackney chain entirely; the hackney 1.x line that httpoison ~> 2.0 pins has several open advisories (SSRF / CRLF / header injection), fixed only in hackney 4.x.
Two ways to clear those advisories — this PR is the larger one, so flagging both:
httpoison ~> 2.0→~> 3.0(httpoison 3.0 uses the patched hackney 4.x). One-line change; happy to send that instead if you prefer to keep httpoison.What changed:
request/5usesReq.request!withdecode_body: false(callersJason.decodethe raw JSON/JOSE body themselves),retry: false(the function already loops on failure), andredirect: false(HTTPoison parity — an ACME POST binds the request URL into the JWS protected header, so a transparently re-POSTed redirect would fail signature validation anyway).Replay-Nonce/Locationresponse headers are read viaReq.Response.get_header/2(Req lowercases response header names).status_code→statusand%HTTPoison.Response{}→%Req.Response{}at all call sites; mixed string/atom request-header keys are normalized for Req.(uri, method)into a(method, uri)head, so every retried request was malformed.httpoison ~> 2.0→req ~> 0.5.One compatibility note: Req's dependency chain (finch) requires Elixir 1.15+, while
mix.exsdeclares~> 1.13. CI (1.15) is unaffected, but merging this effectively raises the supported floor — you may want to bump theelixir:requirement alongside.Verified:
mix test, credo, and format pass; a smoke test against Let's Encrypt staging (directory + newNonce + Replay-Nonce extraction) confirms the exact Req option set works against a real ACME server. Full signed issuance would still be worth a staging run with a live HTTP-01 challenge before a release.