Skip to content

[http-client-csharp] Fix trailing path separator for optional path parameter#11096

Open
JoshLove-msft wants to merge 2 commits into
microsoft:mainfrom
JoshLove-msft:fix/optional-path-trailing-slash
Open

[http-client-csharp] Fix trailing path separator for optional path parameter#11096
JoshLove-msft wants to merge 2 commits into
microsoft:mainfrom
JoshLove-msft:fix/optional-path-trailing-slash

Conversation

@JoshLove-msft

Copy link
Copy Markdown
Contributor

Problem

When a route ends with an optional path parameter, the / separator preceding that segment was emitted unconditionally, leaving a dangling trailing slash when the value is null.

For example, /certificates/{certificate-name}/{certificate-version} (with certificate-version optional) generated:

uri.AppendPath(certificateName, true);
uri.AppendPath("/", false);            // emitted unconditionally
if ((certificateVersion != null))
{
    uri.AppendPath(certificateVersion, true);
}

So when certificateVersion is null the wire URL becomes /certificates/{name}/ instead of /certificates/{name}. Services (e.g. Key Vault) and recorded test cassettes expect no trailing slash.

The existing shouldPrependWithPathSeparator logic only handled the case where the preceding literal does not end in /; it missed the case where the separator had already been written unconditionally.

Fix

In RestClientProvider.AddUriSegments, when the upcoming path parameter is optional and the preceding literal ends with /, defer that trailing / so it is written together with the parameter value inside the null check:

uri.AppendPath(certificateName, true);
if ((certificateVersion != null))
{
    uri.AppendPath("/", false);
    uri.AppendPath(certificateVersion, true);
}

Required path parameters are unaffected.

Tests

  • Added TestBuildCreateRequestMethodWithOptionalPathParameter (+ golden file) covering the trailing optional-segment case.
  • Updated two ServerTemplate* tests that relied on the test factory's default isRequired: false for what were intended to be normal required path parameters.
  • Full Microsoft.TypeSpec.Generator.ClientModel suite passes (1440 tests).

Reported in Azure/azure-sdk-for-net#60162 (Bug A).

--generated by Copilot

When a route ends with an optional path parameter (e.g.
/items/{name}/{version} with optional version), the separator '/'
preceding the optional segment was emitted unconditionally, producing
/items/{name}/ when the value was null instead of /items/{name}.

Defer the trailing separator into the parameter's null check so it is
only written when the optional value is present.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: d425885

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@azure-sdk-automation

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

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.

1 participant