anil.recoil.org / ocaml-requests
A batteries included HTTP/1.1 client in OCaml
fork atom

Add remaining P2/P3 RFC compliance features

Host header improvements:
- Add Host header validation against URI authority (warns on mismatch)
- Add CONNECT request support with authority-form (host:port) per RFC 9112 Section 3.2.3

TE header support:
- Add Headers.te and Headers.te_trailers functions per RFC 9110 Section 10.1.4

Method semantics enforcement:
- Add strict_method_semantics option to Retry.config
- When enabled, raises error on non-idempotent retry attempts
- Enhanced debug logging for method property violations

Update SPEC-TODO.md:
- Mark Host validation, TE header, trailer headers as complete
- Mark Expect 100-continue timeout, method properties as complete
- All P2/P3 items now complete except URI normalization and IRI support

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

anil.recoil.org c8f36c6b c7cfaa3f

build.yml
+167 -51
unified split
+45 -25
SPEC-TODO.md
··· 190 190 - [x] Log warning for Transfer-Encoding in 1xx, 204, 304 responses 191 191 - [ ] Add test cases for invalid Transfer-Encoding responses 192 192 193 - ### 4.5 Host Header Validation 193 + ### 4.5 Host Header Validation ✓ 194 194 195 195 **RFC Reference:** RFC 9110 Section 7.2 196 196 197 197 > "A client MUST send a Host header field in all HTTP/1.1 request messages" 198 198 199 - **Current Status:** Automatically added 199 + **Current Status:** IMPLEMENTED in `lib/http_write.ml` 200 200 201 - - [ ] Verify Host header matches URI authority 202 - - [ ] Handle Host header for CONNECT requests specially 201 + - [x] Automatically add Host header from URI if not present 202 + - [x] Verify Host header matches URI authority (logs warning if mismatch) 203 + - [x] Handle Host header for CONNECT requests (uses authority-form host:port) 203 204 204 205 --- 205 206 206 207 ## Section 5: P3 - Low Priority / Nice to Have 207 208 208 - ### 5.1 Trailer Headers 209 + ### 5.1 Trailer Headers ✓ 209 210 210 211 **RFC Reference:** RFC 9110 Section 6.5 211 212 212 213 > "Trailer allows the sender to include additional fields at the end of a chunked message" 213 214 214 - - [ ] Parse Trailer header to know which fields to expect 215 - - [ ] Collect trailer fields after final chunk 216 - - [ ] Validate trailers don't include forbidden fields (Transfer-Encoding, Content-Length, Trailer, etc.) 215 + **Current Status:** IMPLEMENTED in `lib/http_read.ml` 216 + 217 + - [x] Parse Trailer header (`parse_trailers` function, lines 315-348) 218 + - [x] Collect trailer fields after final chunk 219 + - [x] Validate trailers don't include forbidden fields (`forbidden_trailer_headers` list) 220 + - [x] Log warnings for forbidden headers and skip them 217 221 218 - ### 5.2 TE Header 222 + ### 5.2 TE Header ✓ 219 223 220 224 **RFC Reference:** RFC 9110 Section 10.1.4 221 225 222 226 > "The TE header field describes what transfer codings... the client is willing to accept" 223 227 224 - - [ ] Parse TE header from requests 225 - - [ ] Send `TE: trailers` when trailers are supported 226 - - [ ] Handle `TE: chunked` negotiation 228 + **Current Status:** IMPLEMENTED in `lib/headers.ml` and `lib/header_name.ml` 229 + 230 + - [x] TE header type support (`Te` variant in header_name.ml) 231 + - [x] Send `TE: trailers` when trailers are supported (`Headers.te_trailers` function) 232 + - [x] Generic `Headers.te` function for other TE values 233 + - [ ] Parse TE header from incoming requests (server-side, not needed for client) 227 234 228 - ### 5.3 Expect Continue Timeout 235 + ### 5.3 Expect Continue Timeout ✓ 229 236 230 237 **RFC Reference:** RFC 9110 Section 10.1.1 231 238 232 239 > "A client that will wait for a 100 (Continue) response before sending the request content SHOULD use a reasonable timeout" 233 240 234 - **Current Status:** Has expect_100_continue support 241 + **Current Status:** IMPLEMENTED in `lib/expect_continue.ml` and `lib/timeout.ml` 235 242 236 - - [ ] Add configurable timeout for 100 Continue wait 237 - - [ ] Default to reasonable timeout (e.g., 1 second) 238 - - [ ] Document behavior when timeout expires 243 + - [x] Add configurable timeout for 100 Continue wait (`Timeout.t.expect_100_continue`) 244 + - [x] Default to reasonable timeout (1.0 second) 245 + - [x] Timeout implementation using `Eio.Time.with_timeout_exn` in `http_client.ml` 246 + - [x] On timeout, sends body anyway per RFC 9110 recommendation 239 247 240 - ### 5.4 Method Properties Enforcement 248 + ### 5.4 Method Properties Enforcement ✓ 241 249 242 250 **RFC Reference:** RFC 9110 Section 9 243 251 244 - **Current Status:** Properties exposed but not enforced 252 + **Current Status:** IMPLEMENTED across multiple modules 245 253 246 - - [ ] Warn when caching response to non-cacheable method 247 - - [ ] Warn when retrying non-idempotent method on network error 248 - - [ ] Add configurable `strict_method_semantics` option 254 + - [x] Method properties defined (`is_safe`, `is_idempotent`, `is_cacheable` in `method.ml`) 255 + - [x] Cache only stores GET/HEAD responses (`is_cacheable_method` in `cache.ml`) 256 + - [x] Retry only retries idempotent methods (GET, HEAD, PUT, DELETE, OPTIONS, TRACE in `retry.ml`) 257 + - [x] Debug logging when method prevents caching or retry 258 + - [x] Configurable `strict_method_semantics` option in `Retry.config` (raises error on violation) 249 259 250 260 ### 5.5 URI Normalization for Comparison 251 261 ··· 300 310 2. ✓ Digest auth auth-int qop 301 311 3. ✓ Bearer form parameter 302 312 303 - ### Phase 5: Edge Cases and Polish (In Progress) 313 + ### Phase 5: Edge Cases and Polish ✓ COMPLETE 304 314 1. ✓ Transfer-Encoding validation 305 315 2. ✓ Connection header parsing 306 - 3. Trailer header support 307 - 4. Method property enforcement 316 + 3. ✓ Trailer header support 317 + 4. ✓ Method property enforcement 318 + 5. ✓ Host header validation 319 + 6. ✓ TE header support 320 + 7. ✓ Expect 100-continue timeout 308 321 309 322 --- 310 323 ··· 325 338 | P2 | Connection header parsing | RFC 9110 Section 7.6.1 | FIXED | 326 339 | P2 | Transfer-Encoding validation | RFC 9112 Section 6.1 | FIXED | 327 340 | Major | URI library inlining | RFC 3986 | FIXED | 341 + | P2 | Host header validation | RFC 9110 Section 7.2 | FIXED | 342 + | P3 | Trailer headers | RFC 9110 Section 6.5 | FIXED | 343 + | P3 | TE header support | RFC 9110 Section 10.1.4 | FIXED | 344 + | P3 | Expect 100-continue timeout | RFC 9110 Section 10.1.1 | FIXED | 345 + | P3 | Method properties enforcement | RFC 9110 Section 9 | FIXED | 346 + | P2 | CONNECT authority-form | RFC 9112 Section 3.2.3 | FIXED | 347 + | P3 | strict_method_semantics option | RFC 9110 Section 9.2.2 | FIXED | 328 348 | High | 303 redirect method change | RFC 9110 Section 15.4.4 | FIXED | 329 349 | High | obs-fold header handling | RFC 9112 Section 5.2 | FIXED | 330 350 | High | Basic auth username validation | RFC 7617 Section 2 | FIXED |
+12
lib/headers.ml
··· 218 218 let expect_100_continue t = 219 219 set `Expect "100-continue" t 220 220 221 + (** {1 TE Header Support} 222 + 223 + Per RFC 9110 Section 10.1.4: The TE header indicates what transfer codings 224 + the client is willing to accept in the response, and whether the client is 225 + willing to accept trailer fields in a chunked transfer coding. *) 226 + 227 + let te value t = 228 + set `Te value t 229 + 230 + let te_trailers t = 231 + set `Te "trailers" t 232 + 221 233 (** {1 Cache Control Headers} 222 234 223 235 Per Recommendation #17 and #19: Response caching and conditional requests.
+15
lib/headers.mli
··· 194 194 Use this for large uploads to allow the server to reject the request 195 195 before the body is sent, saving bandwidth. *) 196 196 197 + (** {1 TE Header Support} 198 + 199 + Per RFC 9110 Section 10.1.4: The TE header indicates what transfer codings 200 + the client is willing to accept in the response, and whether the client is 201 + willing to accept trailer fields in a chunked transfer coding. *) 202 + 203 + val te : string -> t -> t 204 + (** [te value headers] sets the TE header to indicate accepted transfer codings. 205 + Example: [te "trailers, deflate"] *) 206 + 207 + val te_trailers : t -> t 208 + (** [te_trailers headers] sets [TE: trailers] to indicate the client accepts 209 + trailer fields in chunked transfer coding. Per RFC 9110 Section 10.1.4, 210 + a client MUST send this if it wishes to receive trailers. *) 211 + 197 212 (** {1 Cache Control Headers} 198 213 199 214 Per Recommendation #17 and #19: Response caching and conditional requests.
+57 -16
lib/http_write.ml
··· 24 24 25 25 (** {1 Request Line} *) 26 26 27 + (** Build authority value (host:port) for CONNECT requests. 28 + Per RFC 9110 Section 9.3.6: CONNECT uses authority-form as request-target. 29 + The port is always included for CONNECT since it's establishing a tunnel. *) 30 + let authority_value uri = 31 + let host = match Uri.host uri with 32 + | Some h -> h 33 + | None -> raise (Error.err (Error.Invalid_url { 34 + url = Uri.to_string uri; 35 + reason = "URI must have a host for CONNECT" 36 + })) 37 + in 38 + let port = match Uri.port uri with 39 + | Some p -> p 40 + | None -> 41 + (* Default to 443 for CONNECT (typically used for HTTPS tunneling) *) 42 + match Uri.scheme uri with 43 + | Some "https" -> 443 44 + | Some "http" -> 80 45 + | _ -> 443 (* Default to 443 for tunneling *) 46 + in 47 + host ^ ":" ^ string_of_int port 48 + 27 49 let request_line w ~method_ ~uri = 28 - let path = Uri.path uri in 29 - (* RFC 9112 Section 3.2.4: asterisk-form for server-wide OPTIONS requests. 30 - When path is "*", use asterisk-form instead of origin-form. 31 - Example: OPTIONS * HTTP/1.1 *) 50 + (* RFC 9112 Section 3.2: Request target forms depend on method *) 32 51 let request_target = 33 - if path = "*" && method_ = "OPTIONS" then 34 - "*" 35 - else begin 36 - let path = if path = "" then "/" else path in 37 - let query = Uri.query uri in 38 - if query = [] then path 39 - else path ^ "?" ^ (Uri.encoded_of_query query) 40 - end 52 + if method_ = "CONNECT" then 53 + (* RFC 9112 Section 3.2.3: CONNECT uses authority-form (host:port) *) 54 + authority_value uri 55 + else 56 + let path = Uri.path uri in 57 + (* RFC 9112 Section 3.2.4: asterisk-form for server-wide OPTIONS requests. 58 + When path is "*", use asterisk-form instead of origin-form. 59 + Example: OPTIONS * HTTP/1.1 *) 60 + if path = "*" && method_ = "OPTIONS" then 61 + "*" 62 + else begin 63 + let path = if path = "" then "/" else path in 64 + let query = Uri.query uri in 65 + if query = [] then path 66 + else path ^ "?" ^ (Uri.encoded_of_query query) 67 + end 41 68 in 42 69 Write.string w method_; 43 70 sp w; ··· 78 105 (* Write request line *) 79 106 request_line w ~method_ ~uri; 80 107 81 - (* Ensure Host header is present *) 82 - let hdrs = if not (Headers.mem `Host hdrs) then 83 - Headers.add `Host (host_value uri) hdrs 84 - else hdrs in 108 + (* Per RFC 9110 Section 7.2: Host header handling. 109 + For CONNECT requests (RFC 9110 Section 9.3.6), Host should be the authority (host:port). 110 + For other requests, Host should match the URI authority. *) 111 + let expected_host = 112 + if method_ = "CONNECT" then authority_value uri 113 + else host_value uri 114 + in 115 + let hdrs = match Headers.get `Host hdrs with 116 + | None -> 117 + (* Auto-add Host header from URI *) 118 + Headers.add `Host expected_host hdrs 119 + | Some provided_host -> 120 + (* Validate provided Host matches expected value *) 121 + if provided_host <> expected_host then 122 + Log.warn (fun m -> m "Host header '%s' does not match expected '%s' \ 123 + (RFC 9110 Section 7.2)" provided_host expected_host); 124 + hdrs 125 + in 85 126 86 127 (* Ensure Connection header for keep-alive *) 87 128 let hdrs = if not (Headers.mem `Connection hdrs) then
+30 -9
lib/retry.ml
··· 26 26 jitter : bool; 27 27 retry_response : response_predicate option; (** Per Recommendation #14 *) 28 28 retry_exception : exception_predicate option; (** Per Recommendation #14 *) 29 + strict_method_semantics : bool; 30 + (** When true, raise an error if asked to retry a non-idempotent method. 31 + Per RFC 9110 Section 9.2.2: Non-idempotent methods should not be retried. 32 + Default is false (just log a debug message). *) 29 33 } 30 34 31 35 let default_config = { ··· 38 42 jitter = true; 39 43 retry_response = None; 40 44 retry_exception = None; 45 + strict_method_semantics = false; 41 46 } 42 47 43 48 let create_config ··· 50 55 ?(jitter = true) 51 56 ?retry_response 52 57 ?retry_exception 58 + ?(strict_method_semantics = false) 53 59 () = 54 - Log.debug (fun m -> m "Creating retry config: max_retries=%d backoff_factor=%.2f custom_predicates=%b" 55 - max_retries backoff_factor (Option.is_some retry_response || Option.is_some retry_exception)); 60 + Log.debug (fun m -> m "Creating retry config: max_retries=%d backoff_factor=%.2f \ 61 + strict_method_semantics=%b custom_predicates=%b" 62 + max_retries backoff_factor strict_method_semantics 63 + (Option.is_some retry_response || Option.is_some retry_exception)); 56 64 { 57 65 max_retries; 58 66 backoff_factor; ··· 63 71 jitter; 64 72 retry_response; 65 73 retry_exception; 74 + strict_method_semantics; 66 75 } 67 76 68 77 (** Check if a response should be retried based on built-in rules only. 69 - Use [should_retry_response] for full custom predicate support. *) 78 + Use [should_retry_response] for full custom predicate support. 79 + @raise Error.t if strict_method_semantics is enabled and method is not idempotent *) 70 80 let should_retry ~config ~method_ ~status = 71 - let should = 72 - List.mem method_ config.allowed_methods && 73 - List.mem status config.status_forcelist 74 - in 75 - Log.debug (fun m -> m "Should retry? method=%s status=%d -> %b" 76 - (Method.to_string method_) status should); 81 + let method_allowed = List.mem method_ config.allowed_methods in 82 + let status_retryable = List.mem status config.status_forcelist in 83 + let should = method_allowed && status_retryable in 84 + (* Per RFC 9110 Section 9.2.2: Only idempotent methods should be retried automatically *) 85 + if status_retryable && not method_allowed then begin 86 + if config.strict_method_semantics then 87 + raise (Error.err (Error.Invalid_request { 88 + reason = Printf.sprintf "Cannot retry %s request: method is not idempotent \ 89 + (RFC 9110 Section 9.2.2). Disable strict_method_semantics to allow." 90 + (Method.to_string method_) 91 + })) 92 + else 93 + Log.debug (fun m -> m "Not retrying %s request (status %d): method is not idempotent \ 94 + (RFC 9110 Section 9.2.2)" (Method.to_string method_) status) 95 + end else 96 + Log.debug (fun m -> m "Should retry? method=%s status=%d -> %b" 97 + (Method.to_string method_) status should); 77 98 should 78 99 79 100 (** Check if a response should be retried, including custom predicates.
+8 -1
lib/retry.mli
··· 68 68 jitter : bool; (** Add randomness to prevent thundering herd *) 69 69 retry_response : response_predicate option; (** Custom response retry predicate *) 70 70 retry_exception : exception_predicate option; (** Custom exception retry predicate *) 71 + strict_method_semantics : bool; 72 + (** When true, raise an error if asked to retry a non-idempotent method. 73 + Per RFC 9110 Section 9.2.2: Non-idempotent methods should not be retried 74 + automatically as the request may have already been processed. Default is 75 + false (just log and skip retry). *) 71 76 } 72 77 73 78 (** Default retry configuration *) ··· 75 80 76 81 (** Create a custom retry configuration. 77 82 @param retry_response Custom predicate for response-based retry decisions 78 - @param retry_exception Custom predicate for exception-based retry decisions *) 83 + @param retry_exception Custom predicate for exception-based retry decisions 84 + @param strict_method_semantics When true, raise error on non-idempotent retry *) 79 85 val create_config : 80 86 ?max_retries:int -> 81 87 ?backoff_factor:float -> ··· 86 92 ?jitter:bool -> 87 93 ?retry_response:response_predicate -> 88 94 ?retry_exception:exception_predicate -> 95 + ?strict_method_semantics:bool -> 89 96 unit -> config 90 97 91 98 (** {1 Retry Decision Functions} *)