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

Fix possible stack overflow in HttpListener #74644

Merged
merged 2 commits into from
Aug 30, 2022

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Aug 26, 2022

Fixes #73674.

This PR replaces mutual recursion between Accept and ProcessAccept by a loop in Accept, which avoids infinite recursion.

@ghost
Copy link

ghost commented Aug 26, 2022

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

Issue Details

null

Author: rzikm
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@rzikm rzikm requested a review from a team August 26, 2022 11:03
rzikm added a commit to dotnet/dotnet-api-docs that referenced this pull request Aug 26, 2022
@rzikm
Copy link
Member Author

rzikm commented Aug 26, 2022

I see that there is a similar problem inside HttpListener.HttpConnection.OnRead, and also between ProcessSend and ProcessReceive in the mentioned example.

Rewriting the HttpConnection may be outside of the scope, because it is in maintenance, but we should probably look at the example in docs.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.
Any chance we can add a test?

And it seems to break some trimming tests for what ever reason.

@rzikm
Copy link
Member Author

rzikm commented Aug 29, 2022

Any chance we can add a test?

I was unable to trigger the stack overflow using my local program, so I don't trying to convert it to a test would be useful.

@rzikm
Copy link
Member Author

rzikm commented Aug 30, 2022

Test failure is #74795

@rzikm rzikm merged commit d564175 into dotnet:main Aug 30, 2022
rzikm added a commit to dotnet/dotnet-api-docs that referenced this pull request Aug 30, 2022
* Update AsyncSockerServer to fix possible StackOverflow

This reflects fixes from dotnet/runtime#74644

* Fix compilation

* Update AsyncSocketServer.cs

* Update snippets/csharp/System.Net.Sockets/SocketAsyncEventArgs/Overview/AsyncSocketServer.cs

Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>

Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
@karelz karelz added this to the 8.0.0 milestone Sep 3, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpListener Stack over flow
4 participants