Skip to content

Conversation

@Tratcher
Copy link
Member

@Tratcher Tratcher commented Apr 26, 2022

Fix MapPath and UsePathBase's use of implicit PathStrings

Fixes a regression caused by the use of implicit converters between string and PathString.

Description

PathString requires that the url path be in it's un-escaped format and defines implicit converters to/from string that will un/escape %XX values. This conversion must only happen once to avoid ambiguity issues like escaped % characters.

A regression happened in 6.0 where methods were split up for performance reasons and a helper method accidentally used string parameters instead of PathStrings. The implicit converter made this compile, but the resulting value was in the wrong format (double un-escaped).

Fixes #41168

Customer Impact

UsePathBase is used by default in some IIS scenarios and the resulting mis-formatted path can cause decoding exceptions and failed requests. This is impacting customers that have upgraded to 6.0. #41168 (comment)

Regression?

  • Yes
  • No

From 5.0

Risk

  • High
  • Medium
  • Low

Corner case that's unit testable.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@Tratcher Tratcher added the Servicing-consider Shiproom approval is required for the issue label Apr 26, 2022
@Tratcher Tratcher added this to the 6.0.x milestone Apr 26, 2022
@Tratcher Tratcher self-assigned this Apr 26, 2022
@ghost ghost added the area-runtime label Apr 26, 2022
@ghost
Copy link

ghost commented Apr 26, 2022

Hi @Tratcher. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@ghost
Copy link

ghost commented Apr 26, 2022

Hi @Tratcher. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@Tratcher Tratcher changed the title Fix MapPath and UsePathBase's use of implicit PathStrings #41168 (#41… Fix MapPath and UsePathBase's use of implicit PathStrings Apr 26, 2022
@Tratcher Tratcher marked this pull request as ready for review April 26, 2022 23:01
@Tratcher Tratcher requested a review from BrennanConroy as a code owner April 26, 2022 23:01
@Tratcher Tratcher added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Apr 28, 2022
@halter73 halter73 modified the milestones: 6.0.x, 6.0.6 May 4, 2022
@halter73
Copy link
Member

halter73 commented May 4, 2022

/azp run

@halter73 halter73 enabled auto-merge (squash) May 4, 2022 22:44
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@halter73 halter73 disabled auto-merge May 4, 2022 23:07
@halter73 halter73 enabled auto-merge (squash) May 4, 2022 23:07
@halter73 halter73 merged commit 7487a1f into dotnet:release/6.0 May 5, 2022
@Tratcher Tratcher deleted the tratcher/release/6.0/pathbase branch May 16, 2022 21:47
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants