From a55010cf337b995c5a0d4654eeaa4efa4865e7b7 Mon Sep 17 00:00:00 2001 From: James Lal Date: Tue, 28 Jul 2026 14:44:35 -0600 Subject: [PATCH] fix bodyless request content length --- .beads/issues.jsonl | 1 + src/client_generator.rs | 29 ++++++-- tests/operation_generation_test.rs | 103 +++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 4 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 911084c..dff00b9 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -43,6 +43,7 @@ {"id":"openapi-generator-cv4","title":"Support exploded query parameters (GH #27)","description":"GH issue 27: query params with style=form, explode=true (OAS defaults) and object schemas currently map to Option\u003cimpl AsRef\u003cstr\u003e\u003e and are sent as one opaque string. Per OAS/RFC6570 form-explode, each object property must become its own query pair (?color=red). Fix: analyzer synthesizes/resolves a typed struct for object query params with form+explode semantics; client generator emits req.query(\u0026struct) so reqwest/serde_urlencoded serializes properties as individual pairs.","notes":"Implemented on branch issue-27-exploded-query-params, draft PR https://github.com/gpu-cli/openapi-to-rust/pull/28. Close when PR merges. Follow-ups: openapi-generator-anu (deepObject/explode=false/arrays), openapi-generator-0jz (server side).","status":"closed","priority":1,"issue_type":"feature","assignee":"James Lal","owner":"james@littlebearlabs.io","created_at":"2026-07-12T23:22:24Z","created_by":"James Lal","updated_at":"2026-07-13T04:21:19Z","started_at":"2026-07-12T23:22:49Z","closed_at":"2026-07-13T04:21:19Z","close_reason":"Released in v0.6.0 (PR #28)","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"openapi-generator-5q8","title":"DateStrategy::Time emits broken serde codec for time::Date/time::Time (GH #25)","description":"GitHub issue gpu-cli/openapi-to-rust#25: fields with format: date/time under DateStrategy::Time get #[serde(with = \"time::serde::iso8601\")], but that module only supports OffsetDateTime — generated code fails to compile. Fix: emit time::serde::format_description! helper codec modules (time_date_format / time_time_format) into generated code, and correct the time dep requirement features (serde alone doesn't even enable rfc3339).","notes":"Fixed in PR https://github.com/gpu-cli/openapi-to-rust/pull/26 (branch worktree-issue-25-time-date-serde). Close when PR merges.","status":"closed","priority":1,"issue_type":"bug","assignee":"James Lal","owner":"james@littlebearlabs.io","created_at":"2026-07-11T18:36:28Z","created_by":"James Lal","updated_at":"2026-07-11T18:57:02Z","started_at":"2026-07-11T18:36:45Z","closed_at":"2026-07-11T18:57:02Z","close_reason":"Fixed in PR #26, merged to main, released in v0.5.3","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"openapi-generator-dpd","title":"Hybrid string-or-object discriminated unions deserialize-fail","description":"When an anyOf/oneOf union contains a discriminator AND a non-object branch (e.g. string-enum like ToolChoiceOptions), the generator emits a tagged enum that cannot deserialize the string form. Real-world hit: OpenAI ToolChoiceParam returns 'auto' in Response.tool_choice, but generated type is #[serde(tag=\"type\")] enum with no untagged String variant. Need to fall back to #[serde(untagged)] when the union mixes string/scalar branches with tagged-object branches, OR add a String fallback variant before the tagged variants.","notes":"Live repro: `ToolChoiceParam` from openai.yaml line 52518. anyOf has 8 branches; the first (`ToolChoiceOptions`) is a string-enum (\"none\"|\"auto\"|\"required\"), the rest are objects with discriminator propertyName=type. Generator emits `#[serde(tag=\"type\")] enum ToolChoiceParam { ToolChoiceOptions(ToolChoiceOptions), ... }` which cannot deserialize the string \"auto\" because serde tries to read a \"type\" field from a JSON string. Fix: when an anyOf/oneOf branch is a non-object schema (string/number/etc), the generator must emit `#[serde(untagged)]` with the scalar branch first OR add a String variant before the tagged variants. Hit on real OpenAI Responses API `Response.tool_choice` field.","status":"closed","priority":1,"issue_type":"bug","assignee":"James Lal","owner":"james@littlebearlabs.io","created_at":"2026-05-10T23:47:35Z","created_by":"James Lal","updated_at":"2026-05-11T00:12:46Z","started_at":"2026-05-10T23:54:10Z","closed_at":"2026-05-11T00:12:46Z","close_reason":"Fixed in src/analysis.rs: (dpd) analyze_oneof_union now downgrades to untagged when any branch is non-object — verified live against OpenAI Response.tool_choice='auto' which now deserializes as ToolChoiceParam::ToolChoiceOptions(Auto). (bgo) merge_schema_into_properties now ORs in is_nullable_pattern() for allOf-merged props — verified live against OpenAI Response.incomplete_details which is now Option\u003cResponseIncompleteDetails\u003e and deserializes null cleanly. All 4 smoke tests (OpenAI+Anthropic, sync+stream) pass.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"openapi-generator-brl","title":"Emit Content-Length zero for bodyless requests","description":"Implement GitHub issue #43 by generating explicit Content-Length: 0 framing for bodyless POST, PUT, and PATCH client operations, including optional bodies passed as None, with regression coverage.","acceptance_criteria":"Generated bodyless POST, PUT, and PATCH requests set Content-Length: 0; methods without content semantics do not; optional bodies only set zero when absent; tests pass; draft PR references issue #43.","notes":"Implemented explicit Content-Length: 0 framing in client codegen for bodyless POST, PUT, and PATCH requests, including schema-less operations and optional bodies passed as None. Added positive, negative-method, schema-less, and optional-body regression tests. cargo fmt --check, cargo clippy --all-features -- -D warnings, and cargo test --all-features pass.","status":"closed","priority":2,"issue_type":"bug","assignee":"James Lal","owner":"james@littlebearlabs.io","created_at":"2026-07-28T20:34:26Z","created_by":"James Lal","updated_at":"2026-07-28T20:43:25Z","started_at":"2026-07-28T20:34:37Z","closed_at":"2026-07-28T20:43:25Z","close_reason":"Implemented and validated; publishing draft PR for GitHub issue #43.","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"openapi-generator-oug","title":"Default(ErrorResponse) error variant is generated but never constructed","description":"Every generated per-operation error enum includes a Default(ErrorResponse) variant for the spec's 'default' response, but no code path ever constructs it: 'grep -c \"ApiError::Default(\"' returns 0 in generated clients.\n\nVERIFIED on the RunPod v2 client: Default(ErrorResponse) is declared (types/client around lines 298, 308, 318 of the generated client) and constructed zero times.\n\nConsequence: a live 422 whose body parses cleanly into ErrorResponse still yields typed: None, so callers fall back to raw body strings. The per-operation typed-error feature — one of the generator's headline selling points — silently does not work for default responses.\n\nFound by live-testing the RunPod v2 API against the generated client.","acceptance_criteria":"A response matched only by the spec's 'default' response constructs the Default(..) variant with the typed body, and a test asserts typed is Some for that case.","status":"closed","priority":2,"issue_type":"bug","owner":"james@littlebearlabs.io","created_at":"2026-07-26T23:51:30Z","created_by":"James Lal","updated_at":"2026-07-27T00:59:39Z","closed_at":"2026-07-27T00:59:39Z","close_reason":"Fixed in fad3ed3; verified against the live RunPod v2 API (list_gpu_types and list_pods now deserialize, with real nulls present in the responses). 362 tests pass.","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"openapi-generator-igg","title":"Default client base_url from servers[0].url when config omits it","description":"HttpClient::new() initializes base_url to String::new() when the TOML config has no [http_client] base_url, even when the OpenAPI document declares servers[0].url. Users must read the docs and supply the base URL manually or every request 404s.\n\nObserved generating from https://api.runpod.io/v2/openapi.json, whose spec declares servers[0].url = https://api.runpod.io; the generated client still starts with an empty base_url.\n\nThis matters most for published, generated client crates, where HttpClient::new() working out of the box is the difference between a crate that feels native and one that appears broken on first use.\n\nFix: when [http_client].base_url is absent, fall back to servers[0].url from the spec. Explicit config keeps precedence. Consider also emitting it as a pub const BASE_URL so downstream crates can reference it without hardcoding a string.","acceptance_criteria":"Generating from a spec with a servers entry and no configured base_url yields a client that targets servers[0].url by default; an explicitly configured base_url still wins.","status":"closed","priority":2,"issue_type":"feature","owner":"james@littlebearlabs.io","created_at":"2026-07-26T23:35:28Z","created_by":"James Lal","updated_at":"2026-07-27T00:59:40Z","closed_at":"2026-07-27T00:59:40Z","close_reason":"Fixed in fad3ed3; verified against the live RunPod v2 API (list_gpu_types and list_pods now deserialize, with real nulls present in the responses). 362 tests pass.","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"openapi-generator-tk1","title":"Cache CI scratch compilation targets","description":"Route install-smoke and spec-compile Cargo target directories into paths retained by the existing rust-cache steps so hosted runners reuse compiled dependencies across runs.","acceptance_criteria":"PR and scheduled spec compile jobs use a stable cached scratch target; install-smoke uses a stable cached install target; temporary packaging and generated fixture cleanup remain isolated; workflow syntax and relevant smoke commands pass.","status":"closed","priority":2,"issue_type":"task","assignee":"James Lal","owner":"james@littlebearlabs.io","created_at":"2026-07-26T22:35:49Z","created_by":"James Lal","updated_at":"2026-07-26T22:40:16Z","started_at":"2026-07-26T22:35:53Z","closed_at":"2026-07-26T22:40:16Z","close_reason":"CI now routes install-smoke and both spec-compile scratch targets through absolute paths under the rust-cache-managed root target tree. actionlint, targeted Anthropic spec compile (offline), and install smoke pass.","dependencies":[{"issue_id":"openapi-generator-tk1","depends_on_id":"openapi-generator-in6","type":"discovered-from","created_at":"2026-07-26T16:35:49Z","created_by":"James Lal","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/src/client_generator.rs b/src/client_generator.rs index c4d034a..1e157f2 100644 --- a/src/client_generator.rs +++ b/src/client_generator.rs @@ -2035,11 +2035,13 @@ impl CodeGenerator { /// Generate request body serialization based on content type /// Emit statements that mutate `req` to apply the request body. Returns - /// `quote!{}` if the operation has no body. Optional bodies (T11) gate the - /// application on `Some(_)`; required bodies apply unconditionally. + /// explicit zero-length framing for bodyless POST, PUT, and PATCH requests. + /// Optional bodies (T11) gate the application on `Some(_)`; required bodies + /// apply unconditionally. fn generate_request_body(&self, op: &OperationInfo) -> TokenStream { + let empty_request_framing = Self::generate_empty_request_framing(op); let Some(rb) = op.request_body.as_ref() else { - return quote! {}; + return empty_request_framing; }; use crate::analysis::RequestBodyContent; let required = op.request_body_required; @@ -2096,7 +2098,7 @@ impl CodeGenerator { }, ) } - RequestBodyContent::SchemaLess { .. } => return quote! {}, + RequestBodyContent::SchemaLess { .. } => return empty_request_framing, }; if required { apply @@ -2104,11 +2106,30 @@ impl CodeGenerator { quote! { if let Some(#ident) = #ident { #apply + } else { + #empty_request_framing } } } } + /// Emit explicit HTTP/1.1 framing when an operation sends no request body. + /// RFC 9110 recommends `Content-Length: 0` for methods that define request + /// content semantics. Methods without those semantics intentionally remain + /// unchanged. + fn generate_empty_request_framing(op: &OperationInfo) -> TokenStream { + if ["POST", "PUT", "PATCH"] + .iter() + .any(|method| op.method.eq_ignore_ascii_case(method)) + { + quote! { + req = req.header(reqwest::header::CONTENT_LENGTH, "0"); + } + } else { + quote! {} + } + } + /// Find the success (2xx) response schema name, if any. /// /// Only considers 2xx status codes. Error schemas (4xx, 5xx) are ignored diff --git a/tests/operation_generation_test.rs b/tests/operation_generation_test.rs index 632317c..6719a63 100644 --- a/tests/operation_generation_test.rs +++ b/tests/operation_generation_test.rs @@ -146,6 +146,109 @@ fn schema_less_request_content_preserves_client_operation_without_a_body() { assert!(result.contains(". post (request_url)"), "{result}"); assert!(!result.contains("request :"), "{result}"); assert!(!result.contains(". body ("), "{result}"); + assert!( + result.contains("reqwest :: header :: CONTENT_LENGTH , \"0\""), + "{result}" + ); +} + +#[test] +fn bodyless_content_methods_emit_zero_content_length() { + let config = create_test_config(); + let generator = CodeGenerator::new(config); + + for method in ["POST", "PUT", "PATCH"] { + let operation = OperationInfo { + operation_id: format!("bodyless{method}"), + method: method.to_string(), + path: "/actions".to_string(), + summary: None, + description: None, + request_body: None, + response_schemas: BTreeMap::new(), + parameters: vec![], + request_body_required: false, + supports_streaming: false, + stream_parameter: None, + tags: Vec::new(), + }; + + let analysis = create_test_analysis_with_operations(vec![operation]); + let result = generator.generate_operation_methods(&analysis).to_string(); + + assert!( + result.contains("reqwest :: header :: CONTENT_LENGTH , \"0\""), + "expected Content-Length: 0 for {method}: {result}" + ); + } +} + +#[test] +fn bodyless_methods_without_content_semantics_do_not_emit_content_length() { + let config = create_test_config(); + let generator = CodeGenerator::new(config); + + for method in ["GET", "HEAD", "DELETE", "OPTIONS", "TRACE"] { + let operation = OperationInfo { + operation_id: format!("bodyless{method}"), + method: method.to_string(), + path: "/actions".to_string(), + summary: None, + description: None, + request_body: None, + response_schemas: BTreeMap::new(), + parameters: vec![], + request_body_required: false, + supports_streaming: false, + stream_parameter: None, + tags: Vec::new(), + }; + + let analysis = create_test_analysis_with_operations(vec![operation]); + let result = generator.generate_operation_methods(&analysis).to_string(); + + assert!( + !result.contains("reqwest :: header :: CONTENT_LENGTH"), + "did not expect Content-Length for {method}: {result}" + ); + } +} + +#[test] +fn optional_content_body_emits_zero_content_length_only_when_absent() { + let config = create_test_config(); + let generator = CodeGenerator::new(config); + let operation = OperationInfo { + operation_id: "maybeCreateUser".to_string(), + method: "POST".to_string(), + path: "/users".to_string(), + summary: None, + description: None, + request_body: Some(RequestBodyContent::Json { + schema_name: "CreateUserRequest".to_string(), + media_type: "application/json".to_string(), + validation_schema: serde_json::Value::Null, + }), + response_schemas: BTreeMap::new(), + parameters: vec![], + request_body_required: false, + supports_streaming: false, + stream_parameter: None, + tags: Vec::new(), + }; + + let analysis = create_test_analysis_with_operations(vec![operation]); + let result = generator.generate_operation_methods(&analysis).to_string(); + + assert!( + result.contains("if let Some (request) = request"), + "{result}" + ); + assert!(result.contains("else { req = req . header"), "{result}"); + assert!( + result.contains("reqwest :: header :: CONTENT_LENGTH , \"0\""), + "{result}" + ); } #[test]