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

Adding RestoreRawRequestPath middlware to restore the raw request path value to current request #9402

Closed
wants to merge 3 commits into from

Conversation

kshyju
Copy link
Member

@kshyju kshyju commented Jul 19, 2023

Fixes #9290

The current behavior is caused by the App service front end decoding the encoded request path value before forwarding the request to the application (in this case functions host instance). When doing this, the front end stores the raw request path value it received and add it as a special request header.

Below are some relevant comments from other thread where this issue was discussed before.

crosenblatt commented on Oct 14, 2022
Hi @ccromer - This behavior is present in App Service for backwards compatibility reasons. In order to get the raw bytes of the url exactly as they are received on the wire, you can use the header "X-Waws-Unencoded-Url".

and

bradygaster commented on Mar 30, 2022
After corresponding with the service team, I learned that this is By Design: IIS FE's URL parse the request URL during proxying, so the URI will be modified (canonicalized and normalized).

The work around is for the app to use the HTTP_X_WAWS_UNENCODED_URL server variable (a.k.a. X-WAWS-Unencoded-URL: header). This header contains the exact bytes as they were received from the wire.

The fix here is to use the X-Waws-Unencoded-Url header value which has the raw request path (un decoded) and update the request path with that so that the routing middleware can use that and route to the correct endpoint. I added a new middleware to do this. This is an opt-in feature customers needs to explicitly opt-in by setting the WEBSITE_RESTORE_RAW_REQUEST_PATH app setting.

This issue is a minimal repro of the original issue.

Pull request checklist

  • My changes do not require documentation changes
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

Additional information

…o the request context. This is an opt-in feature customers needs to explicitly opt-in.
@kshyju
Copy link
Member Author

kshyju commented Jul 20, 2023

Tested the build from this branch via private site extension. Created 2 sites, both has same code (changes from this PR). One has the feature enabled (WEBSITE_RESTORE_RAW_REQUEST_PATH app setting is present).

Site without the feature:

https://shkr-9290-a-202307181948.azurewebsites.net/api/departments/cloud%2Fdevdiv - 404 - Routing failed.

Site with the feature enabled:

https://shkr-9290-b-202307182000.azurewebsites.net/api/departments/cloud%2Fdevdiv - 200 OK - Routing worked.

Co-authored-by: sarah  <35204912+satvu@users.noreply.github.com>
@kshyju
Copy link
Member Author

kshyju commented Aug 23, 2023

@mattchenderson Let me know how do we want to proceed forward with this change? Did we hear any thing further from the platform folks?

@MitchBodmer
Copy link

Any updates here? It looks code complete but I gather you're waiting on some input from another team?

@kshyju
Copy link
Member Author

kshyju commented Oct 4, 2023

Closing this function level fix as we got confirmation from Antares team that this can be fixed on the platform level.

https://msazure.visualstudio.com/Antares/_workitems/edit/25263505 is the (internal) work item tracking the fix.

@kshyju kshyju closed this Oct 4, 2023
@johan-lindqvist
Copy link

Any way for external interested parties to track the platform level fix?

@kwasilka
Copy link

I'm also eager for a fix for this. I'm experiencing the issue with API's hosted from App Services. I need a fix for this asap

@MitchBodmer
Copy link

Hey @kshyju, I just talked to the team that would be fixing this for the platform and they said they wouldn't be able to get to this until mid/late 2024 at the earliest. Can we revisit this PR as a stopgap and remove it whenever they get around to fixing it? I think this has a lot of value.

@MitchBodmer
Copy link

Also unfortunately they have no clear way of tracking the change publicly.

@kshyju
Copy link
Member Author

kshyju commented Oct 12, 2023

@mattchenderson Can you advice?

@liliankasem liliankasem deleted the shkr/handle-decoded-routeparams branch May 3, 2024 17:24
@mcrio
Copy link

mcrio commented Aug 28, 2024

Hi. Any way to have a workaround for this problem when hosting a DotNet web app with Azure App Services? It's a release blocker and we would never expect the azure internal proxy to magically alter parts of the original URL.

@ccromer
Copy link

ccromer commented Aug 28, 2024 via email

@j9850s
Copy link

j9850s commented Oct 9, 2024

One issue..
The X-Waws-Unencoded-Url contains the PathAndQuery.
The Request.Path must be set with only the AbsolutePath.

if (context.Request.Headers.TryGetValue(UnencodedUrlHeaderName, out var unencodedUrlValue) && unencodedUrlValue.Any())
{
    var uri = new Uri($"https://fake.url{unencodedUrlValue.First()}");
    context.Request.Path = new PathString(uri.AbsolutePath);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Routing fails if parameters contains slash
10 participants