Skip to content

Fix optional request body to wrap Content-Type header in null check#10079

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/support-optional-request-body
Draft

Fix optional request body to wrap Content-Type header in null check#10079
Copilot wants to merge 4 commits intomainfrom
copilot/support-optional-request-body

Conversation

Copy link
Contributor

Copilot AI commented Mar 18, 2026 •
edited
Loading

When the request body is optional, the C# generator was unconditionally setting the Content-Type header, producing invalid code like if ("application/xml" != null). The Content-Type header should be gated on the content parameter being non-null. The request.Content assignment does not need wrapping since setting it to null is a no-op.

Before:

if ("application/xml" != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
}
request.Content = content;

After:

if (content != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
}
request.Content = content;

Changes

  • Tests: Rewrote ContentTypeHeaderWrappedInNullCheckWhenContentIsOptional and ContentTypeHeaderNotWrappedInNullCheckWhenContentIsRequired to use TestData pattern with MockClientProvider + TypeProviderWriter + Helpers.GetExpectedFromFile(), comparing the full generated CreateRequest method against expected output files
  • TestData: Added ContentTypeHeaderWrappedInNullCheckWhenContentIsOptional.cs and ContentTypeHeaderNotWrappedInNullCheckWhenContentIsRequired.cs TestData files showing the isolated CreateRequest method output for each scenario
Original prompt

This section details on the original issue you should resolve

<issue_title>Support Optional Request Body & Optional Content Type</issue_title>
<issue_description>Consider the following spec : link

Here the request body and the content type header are optional. The content type header should only be added to the request if the request body is not empty. Currently, we generate invalid code that will always add the content type parameter:

if ("application/xml" != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
}

Instead, we should check if the request content is null, and if not, apply the content type:

if (content != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
    request.Content = content;
}
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@jorgerangel-msft
fix: wrap Content-Type header and content assignment in null check fo…
47ad15a 
…r optional body

When the request body is optional, both the Content-Type header setting and
the request.Content assignment are now wrapped in an if (content != null)
block. This prevents setting Content-Type when no body is provided.

For optional body without Content-Type header, the content assignment
alone is wrapped in a null check.

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Mar 18, 2026
@jorgerangel-msft
Copy link
Contributor

jorgerangel-msft commented Mar 18, 2026

@copilot for context, this was suppose to be fixed in a previous change #9521.

Copilot AI changed the title [WIP] Add support for optional request body and content type Fix optional request body to wrap Content-Type and content assignment in null check Mar 18, 2026
Copilot AI requested a review from jorgerangel-msft March 18, 2026 23:25
Copy link
Contributor Author

Copilot AI commented Mar 18, 2026

@copilot for context, this was suppose to be fixed in a previous change #9521.

Thanks for the context. PR #9521 wrapped the Content-Type header in if (content != null) but left request.Content = content unconditionally outside that block. This PR (commit 47ad15a) extends that fix by moving the content assignment inside the same null check, so both the header and content are only set when the body is present.

jorgerangel-msft
jorgerangel-msft reviewed Mar 18, 2026
View reviewed changes
...sharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs Outdated
var contentParam = parameters.FirstOrDefault(
p => p.Location == ParameterLocation.Body);
return contentParam is null ? [] : [request.Content().Assign(contentParam).Terminate()];
if (contentParam is null)
Copy link
Contributor

@jorgerangel-msft jorgerangel-msft Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot actually, we can simplify this. Instead of generating:

if (content != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
    request.Content = content;
}

Lets simply conditionally add the content type header:

if (content != null)
{
    request.Headers.SetValue("Content-Type", "application/xml");
}

The request.Content property can be both null or not, so conditionally checking it is a no-op

Copy link
Contributor Author

Copilot AI Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the source changes in 067ed6f. RestClientProvider.cs is back to the PR #9521 behavior — only the Content-Type header is wrapped in if (content != null), and request.Content = content stays unconditional.

...TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs Outdated
var statements = createMethod!.BodyStatements as MethodBodyStatements;
Assert.IsNotNull(statements);

var expectedStatement = @"if ((content != null))
Copy link
Contributor

@jorgerangel-msft jorgerangel-msft Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot lets use TestData to compare the generated CreateRequest method. We should filter out all the other generated methods to isolate the generated code for the tests

Copy link
Contributor Author

Copilot AI Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 067ed6f. Both tests now use MockClientProvider + TypeProviderWriter + Helpers.GetExpectedFromFile() with TestData files that contain the isolated CreateRequest method output.

Copilot AI and others added 2 commits March 18, 2026 23:41
@jorgerangel-msft
revert: undo source and test changes to address reviewer feedback
b959b70 
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@jorgerangel-msft
refactor: rewrite Content-Type tests to use TestData pattern per revi…
067ed6f 
…ew feedback

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title Fix optional request body to wrap Content-Type and content assignment in null check Fix optional request body to wrap Content-Type header in null check Mar 18, 2026
Copilot AI requested a review from jorgerangel-msft March 18, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgerangel-msft jorgerangel-msft Awaiting requested review from jorgerangel-msft jorgerangel-msft is a code owner

@m-nash m-nash Awaiting requested review from m-nash m-nash will be requested when the pull request is marked ready for review m-nash is a code owner

@JoshLove-msft JoshLove-msft Awaiting requested review from JoshLove-msft JoshLove-msft will be requested when the pull request is marked ready for review JoshLove-msft is a code owner

@christothes christothes Awaiting requested review from christothes christothes will be requested when the pull request is marked ready for review christothes is a code owner

@jsquire jsquire Awaiting requested review from jsquire jsquire will be requested when the pull request is marked ready for review jsquire is a code owner

@ShivangiReja ShivangiReja Awaiting requested review from ShivangiReja ShivangiReja will be requested when the pull request is marked ready for review ShivangiReja is a code owner

@joseharriaga joseharriaga Awaiting requested review from joseharriaga joseharriaga will be requested when the pull request is marked ready for review joseharriaga is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@jorgerangel-msft jorgerangel-msft

Copilot code review Copilot

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support Optional Request Body & Optional Content Type

2 participants

@jorgerangel-msft