Skip to content

Add support for setting security attributes on Http.Sys RequestQueue #61325

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add support for setting security attributes on Http.Sys RequestQueue #61325

wants to merge 4 commits into from

Conversation

BrennanConroy
Copy link
Member

Fixes #61084

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Apr 4, 2025
@BrennanConroy BrennanConroy force-pushed the brecon/requestqueuesecurity branch from 10ebaa7 to cabcc2f Compare April 18, 2025 17:18
@BrennanConroy BrennanConroy marked this pull request as ready for review April 18, 2025 17:22
captainsafia
captainsafia reviewed Apr 18, 2025
View reviewed changes
src/Servers/HttpSys/src/NativeInterop/RequestQueue.cs
@@ -143,6 +174,9 @@ public void Dispose()
}

_disposed = true;

PInvoke.HttpCloseRequestQueue(Handle);
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a reasonable change but unrelated to this PR. Drive-by bug fix or is it related somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

Drive-by, was just reading the docs for HttpCreateRequestQueue to make sure I was doing the correct thing with the security attribute and noticed it said you should close the queue handle with HttpCloseRequestQueue.

@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@captainsafia captainsafia captainsafia left review comments

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@JamesNK JamesNK Awaiting requested review from JamesNK JamesNK is a code owner

@mgravell mgravell Awaiting requested review from mgravell mgravell is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-httpsys pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for setting security attributes on Http.Sys RequestQueue
2 participants
@BrennanConroy @captainsafia