Conversation
A header parameter declared with `content: application/json` was serialized through `ClientUtils.ParameterToString(...)`, which for a model object falls through to `Convert.ToString(obj, ...)` and emits the model's debug `ToString()` representation instead of JSON. This affects every C# client library. Mirror the existing `queryIsJsonMimeType` mechanism: add a shared `headerIsJsonMimeType` flag to CodegenParameter, set in DefaultCodegen.fromParameter via the existing isJsonMimeType(contentType) helper. The C# api templates use it to serialize JSON-content headers as JSON instead of via ParameterToString, keeping existing behavior for regular headers: - restsharp, httpclient, unityWebRequest (Newtonsoft): a new ClientUtils.ParameterToJsonString helper producing ASCII-safe JSON. - generichost (System.Text.Json): JsonSerializer.Serialize(value, _jsonSerializerOptions), consistent with how it serializes bodies. Adds a 3.1 fixture and regression tests for the restsharp and generichost serialization backends. Fixes OpenAPITools#25082
Adds the ClientUtils.ParameterToJsonString helper to the generated Newtonsoft-based clients (restsharp, httpclient, unityWebRequest).
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 32 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- ParameterToJsonString: escape every character >= U+007F (including DEL) as \uXXXX by post-processing the final JSON string, mirroring the Java fix. This is more robust than Newtonsoft's EscapeNonAscii, which leaves DEL (U+007F) unescaped and is bypassed by converters that emit raw JSON. - DefaultCodegen: also treat vendor JSON media types (application/vnd.*+json) as JSON headers via isJsonVendorMimeType, so e.g. application/vnd.acme+json is serialized as JSON instead of ToString(). - Apply the JSON-aware serializer to constant (autoset) header parameters in the restsharp, httpclient and generichost templates. - Extend the fixture with a vendor +json header and add regression tests for the vendor and constant-header cases. Addresses review feedback on OpenAPITools#25083.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 31 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- ParameterToJsonString: serialize with JsonConvert.SerializeObject directly so a null value becomes the JSON literal "null" again (reusing Serialize regressed this to a C# null), and escape every character < U+0020 (C0 controls incl. CR/LF) in addition to >= U+007F. Escaping raw control characters prevents header splitting/injection if a converter emits raw JSON control bytes. - Constant (autoset) JSON headers now render the constant as its schema type: a string constant serializes to a JSON string, an integer/boolean constant to a JSON number/boolean (previously always a quoted string). - Extend the constant fixture/test to cover both a string and an integer constant JSON header. Addresses review feedback on OpenAPITools#25083.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 29 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Revert the JSON-aware serialization of autoset constant header parameters. An autoset (single fixed enum value) header that also declares content: application/json is not a real-world scenario, and correctly serializing arbitrary scalar constant types as JSON source literals (string vs number vs boolean, plus C# literal escaping) adds disproportionate template complexity. Constants keep the existing ParameterToString behavior. The reported bug (variable JSON headers) remains fixed. - Update the ParameterToJsonString doc comment to note that C0 control characters (including CR/LF) are escaped too. Addresses review feedback on OpenAPITools#25083.
This branch has not been deployed
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.
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.
Fixes #25082
Problem
A header parameter declared with
content: application/jsonwas serialized in the generated C# client throughClientUtils.ParameterToString(...). For a model object,ParameterToStringfalls through toConvert.ToString(obj, CultureInfo.InvariantCulture), which emits the model's debugToString():instead of JSON (
{"path":"/x"}). This affects all C# client libraries, since they share this header serialization path:restsharp(default),httpclient,generichost, andunityWebRequest.This is the C# counterpart of the Java bug #25055 (PR #25056) and the Python bug #25067 (PR #25068).
Fix
Mirror the existing
queryIsJsonMimeTypemechanism (which already does exactly this for query params):CodegenParameter.headerIsJsonMimeTypeflag, set inDefaultCodegen.fromParametervia the already-existingisJsonMimeType(contentType)helper. (This is the same small engine addition proposed in fix(java): serialize JSON header parameter content #25056; if that merges first, this can be de-duped.)api.mustachetemplates serialize JSON-content headers as JSON instead of viaParameterToString, keeping existing behavior for regular headers:restsharp,httpclient,unityWebRequest(Newtonsoft): a new ASCII-safeClientUtils.ParameterToJsonString(...)helper (usesStringEscapeHandling.EscapeNonAscii).generichost(System.Text.Json):JsonSerializer.Serialize(value, _jsonSerializerOptions), consistent with how it already serializes request bodies.Generated code for a JSON-content header now looks like:
while a regular header is unchanged (
ParameterToString(xPlainArg)).Tests
3_1/csharp/json-header-content.yaml(a JSON-content header + a plain header).testJsonContentHeaderUsesJsonSerialization(restsharp) andtestJsonContentHeaderUsesJsonSerializationGenericHosttoCSharpClientCodegenTest, asserting the JSON header is JSON-serialized and the plain header is not.Verification
CSharpClientCodegenTest— both new tests pass (Tests run: 2, Failures: 0).restsharp,httpclient, andgenerichostoutputs compile withdotnet build(0 errors).unityWebRequestoutput generates the expected code (requires the Unity SDK to compile).ClientUtils.ParameterToJsonStringhelper added to the Newtonsoft-based generated clients.PR checklist
./mvnw clean packagefor the generator and regenerated the affected samples with./bin/generate-samples.sh bin/configs/csharp*.yaml; committed all changed files.master.cc C# technical committee: @mandrean @shibayan @Blackclaws @lucamazzanti
Summary by cubic
Fixes the C# generator so header parameters declared with
content: application/jsonserialize as JSON instead of the model's debugToString()output, which affected all C# client libraries.Adds a
headerIsJsonMimeTypeflag toCodegenParameter, set inDefaultCodegen.fromParameterfor both JSON and vendor+jsonmedia types, mirroring the existing query-parameter mechanism.restsharp,httpclient, andunityWebRequesttemplates use a newClientUtils.ParameterToJsonStringhelper for JSON-content headers;generichostusesJsonSerializer.Serialize(value, _jsonSerializerOptions). Regular headers keep usingParameterToString.ParameterToJsonStringserializes directly withJsonConvert.SerializeObjectand escapes every character below U+0020 or at/above U+007F as\uXXXXto keep header values free of control characters.restsharpandgenerichostbackends covering JSON, vendor+json, plain, and constant headers.Written for commit 834a599. Summary will update on new commits.