Skip to content

[http-client-csharp] Fix protocol method call-site argument order#11097

Merged
JoshLove-msft merged 2 commits into
microsoft:mainfrom
JoshLove-msft:joshlove/fix-60160-updatesecret-argorder
Jun 29, 2026
Merged

[http-client-csharp] Fix protocol method call-site argument order#11097
JoshLove-msft merged 2 commits into
microsoft:mainfrom
JoshLove-msft:joshlove/fix-60160-updatesecret-argorder

Conversation

@JoshLove-msft

Copy link
Copy Markdown
Contributor

Summary

Fixes the generated C# (@typespec/http-client-csharp) protocol method call site passing arguments to the CreateRequest builder in the wrong positional order.

Reported in Azure/azure-sdk-for-net#60160 for Key Vault updateSecret / updateCertificate.

Root cause

The protocol method orders its parameters required-first (optional parameters and the request options/context parameter are moved to the end so they can carry default values). The CreateRequest builder keeps the operation declaration order. When an optional parameter appears before a required body parameter, these two orders diverge.

The call site forwarded the protocol method's parameters to CreateRequest positionally, so the arguments bound to the wrong builder parameters. For Key Vault updateSecret this routed the RequestContent into the secret-version URL path slot:

// builder (correct)
internal HttpMessage CreateUpdateSecretRequest(string secretName, string secretVersion, RequestContent content, RequestContext context)

// call site (bug) — content and secretVersion swapped
this.CreateUpdateSecretRequest(secretName, content, secretVersion, context);

The resulting PATCH URL became /secrets/{name}/{garbage-content-bytes} and 404'd.

Fix

Map each CreateRequest argument to the protocol parameter with the same name, so values are passed in the order the builder expects (falling back to the previous positional behavior if names can't be reconciled).

Tests

  • New regression test ProtocolMethodCallsCreateRequestWithArgumentsInBuilderOrder reproduces the optional-path-before-required-body scenario and verifies the call site uses the builder's order.
  • Updated the TestMultipartClient_UploadMethods_OptionalBody baseline: it previously showed the sync and async Upload overloads passing arguments to the same CreateUploadRequest builder in different orders — i.e. the same latent bug. Both now correctly use (content, contentType, options).
  • Full Microsoft.TypeSpec.Generator.ClientModel suite passes (1440 tests).

Fixes Azure/azure-sdk-for-net#60160

…0160)

When an optional parameter (e.g. an optional path parameter) appears before a required body parameter, the protocol method orders its parameters required-first, which differs from the CreateRequest builder's declaration order. The call site forwarded the protocol parameters positionally, binding them to the wrong CreateRequest parameters (swapping the request body with the optional parameter) and producing malformed request URLs.

Map each CreateRequest argument to the protocol parameter with the same name so values are passed in the order the builder expects.

Co-authored-by: Copilot <223556219+Copilot@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 Jun 26, 2026
@azure-sdk-automation

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pkg-pr-new

pkg-pr-new Bot commented Jun 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@11097

commit: a2e6fab

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@JoshLove-msft JoshLove-msft enabled auto-merge June 29, 2026 18:56
@JoshLove-msft JoshLove-msft added this pull request to the merge queue Jun 29, 2026
Merged via the queue into microsoft:main with commit 5d609c3 Jun 29, 2026
29 checks passed
@JoshLove-msft JoshLove-msft deleted the joshlove/fix-60160-updatesecret-argorder branch June 29, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[http-client-csharp emitter] UpdateSecret/UpdateCertificate protocol-method call sites pass arguments in wrong positional order

3 participants