Skip to content

[release/5.0] Fix null ref in UrlGroup.Dispose due to logger not set #28494

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

Merged
merged 2 commits into from
Dec 9, 2020

Conversation

shirhatti
Copy link
Contributor

@shirhatti shirhatti commented Dec 8, 2020

Backporting #26983 to 5.0

Description

When disposing a DelegationRule, if the call to HttpCloseUrlGroup within UrlGroup.Dispose() fails then a NullReferenceException happens because _logger is not initialized in the constructor used by the HttpSysRequestDelegationFeature.

This could happen if the delegatee process crashes.

Customer impact

Customer needs to guard against the null ref with a try...catch.
Backport requested by @NGloreous. Has been validated (and submitted by him 😄)

Regression

No. This is a new feature introduced in 5.0

Risk

Very low.

@Tratcher
Copy link
Member

Tratcher commented Dec 8, 2020

Missed some commits?

@shirhatti shirhatti force-pushed the shirhatti/nullrefinurlgroupdispose branch from d9759eb to 73f4a53 Compare December 8, 2020 00:16
@shirhatti
Copy link
Contributor Author

Missed some commits?

Ahh. I always confuse cherry-pick A^..B for just A..B 😅

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why we squash 😁

@shirhatti shirhatti added this to the 5.0.2 milestone Dec 8, 2020
@shirhatti shirhatti added the Servicing-consider Shiproom approval is required for the issue label Dec 8, 2020
@ghost
Copy link

ghost commented Dec 8, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@shirhatti shirhatti changed the base branch from master to release/5.0 December 8, 2020 17:30
@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Dec 8, 2020
@wtgodbe
Copy link
Member

wtgodbe commented Dec 9, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@wtgodbe wtgodbe merged commit 050c257 into release/5.0 Dec 9, 2020
@wtgodbe wtgodbe deleted the shirhatti/nullrefinurlgroupdispose branch December 9, 2020 21:00
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 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.

6 participants