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

[release/6.0] Update Proxy-Support check to be case insensitive #79620

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 13, 2022

Backport of #61446 to release/6.0

Fixes #61414

/cc @karelz @ChrisFWood

Customer Impact

A customer is in the middle of swapping internal proxy/load balancer. The new proxy does not play well with .NET applications, because it uses different casing of Session-Based-Authentication header (header used by proxies), which is valid according to RFC, but .NET does not recognize it. As a result, 6,500 internal .NET applications are not working properly and the new proxy roll out is blocked.

The fix is already part of .NET 7.0, but the customer has internal requirement to stay on LTS, therefore asks for 6.0 backport.

It is not a regression, it has been broken this way since .NET Core 2.0 when we introduced SocketsHttpHandler.

Testing

CI only
Working with the customer on private E2E validation, but it is not clear if it will be possible

Risk

Low, it is limited to proxy handling code paths only.
If the header was not recognized before, the application would not work properly, so the risk of breaking customers depending on broken scenario is low.

RFC4559 does not specify that the Proxy-Support header value used to
determine if the proxy server will honour client server authentication
integrity is case sensitive. Updating the check to be case insensitive
to prevent failures when value is supplied using differences in case.

Fix #61414
@ghost
Copy link

ghost commented Dec 13, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #61446 to release/6.0

/cc @karelz @ChrisFWood

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@teo-tsirpanis teo-tsirpanis added this to the 6.0.x milestone Dec 13, 2022
@karelz karelz added the Servicing-consider Issue for next servicing release review label Dec 16, 2022
@karelz
Copy link
Member

karelz commented Dec 20, 2022

Test failures are unrelated infra problems, happening also on rerun:

  • Helix queue for .NET 4.8 was removed - [6.0] .NET 4.8 queue removed #79858
    • SENDHELIXJOB(0,0): error : Helix queue windows.10.amd64.client21h1.open was set for estimated removal date of 2022-12-13. In most cases the queue will be removed permanently due to end-of-life; please contact dnceng for any questions or concerns, and we can help you decide how to proceed and discuss other options.
  • McCatalyst Mono failure to build app - [6.0][7.0] MacCatalyst Mono - XcodeBuildApp failed moving directory #79859
    • src/libraries/tests.proj(416,5): error MSB4018: The "XcodeBuildApp" task failed unexpectedly. System.IO.DirectoryNotFoundException: Could not find a part of the path

@karelz karelz added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 20, 2022
@karelz
Copy link
Member

karelz commented Dec 20, 2022

Approved via email by @SteveMCarroll on Sat 2022/12/17 1:00 AM -- adding Servicing-approved label

  • email: "[6.0.x] Update Proxy-Support check to be case insensitive"

@karelz karelz self-assigned this Jan 3, 2023
@carlossanlop carlossanlop modified the milestones: 6.0.x, 6.0.14 Jan 5, 2023
@carlossanlop
Copy link
Member

Approved by Tactics (6.0.14).
Signed off by area owners.
No OOB changes needed - The involved src csproj does not have IsPackable=true.
CI failures investigated as unrelated.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 756342b into release/6.0 Jan 5, 2023
@jkotas jkotas deleted the backport/pr-61446-to-release/6.0 branch January 6, 2023 05:36
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants