Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dotnet-getdocument invocation, improper path handling on Windows in Microsoft.Extensions.ApiDescription.Server.targets breaks build #59749

Open
1 task done
dlosch opened this issue Jan 7, 2025 · 2 comments · May be fixed by #59753
Labels
area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI feature-openapi

Comments

@dlosch
Copy link

dlosch commented Jan 7, 2025

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The microsoft.extensions.apidescription.server NuGet in src\Tools\Extensions.ApiDescription.Server\src\build\Microsoft.Extensions.ApiDescription.Server.targets has an improper handling of directory separators when executing the dotnet-getdocument.dll tool on Windows.

This breaks the build on Windows, if the value of OpenApiDocumentsDirectory ends in a Linux-style directory separator (/)

microsoft.extensions.apidescription.server NuGet version: 9.0.0

Thanks.

Expected Behavior

Build should behave the same on Windows and Linux.

Steps To Reproduce

Bug occurs when

  • Build runs on Windows
  • OpenApiDocumentsDirectory has a path ending with a Linux-style directory separator /, for example <OpenApiDocumentsDirectory>../openapi/</OpenApiDocumentsDirectory> in my .csproj

Where and why does the bug occur
File: src\Tools\Extensions.ApiDescription.Server\src\build\Microsoft.Extensions.ApiDescription.Server.targets

    <PropertyGroup>
      <_DotNetGetDocumentOutputPath>$(OpenApiDocumentsDirectory.TrimEnd('\'))</_DotNetGetDocumentOutputPath>
      <_DotNetGetDocumentOutputPath>$([System.IO.Path]::GetFullPath('$(_DotNetGetDocumentOutputPath)'))</_DotNetGetDocumentOutputPath>

This trims the trailing separator if and only the separator already is a Windows-style backslash. If it is a Linux style /, nothing gets trimmed. the Linux style path ../openapi/ gets translated to a Windows style path by $([System.IO.Path]::GetFullPath. This results in _DotNetGetDocumentOutputPath containing a path with a trailing \, which breaks the command line the target generates (because it results in \", escaping the closing ").

Exceptions (if any)

Proposed Fix:
in src\Tools\Extensions.ApiDescription.Server\src\build\Microsoft.Extensions.ApiDescription.Server.targets, the .TrimEnd('\') needs to go after the $([System.IO.Path]::GetFullPath('$(_DotNetGetDocumentOutputPath)')), because the latter translates / to \ on windows. Hence if the initial path ends with /, nothing gets trimmed, then GetFullPath translates the / to \ on windows, and the command line contains an escaped " (\").

.NET Version

9.0.200-preview.0.24575.35

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jan 7, 2025
@martincostello martincostello added area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI feature-openapi and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Jan 7, 2025
@martincostello
Copy link
Member

Would you like to submit a PR with your proposed fix?

dlosch added a commit to dlosch/aspnetcore that referenced this issue Jan 7, 2025
@dlosch dlosch linked a pull request Jan 7, 2025 that will close this issue
4 tasks
@dlosch
Copy link
Author

dlosch commented Jan 7, 2025

Based on the previous commits it looks like a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI feature-openapi
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants