Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Fix Mono bug #52508 in HttpListener when doing multiple Https request. #19487

Merged
merged 1 commit into from
May 11, 2017

Conversation

baulig
Copy link

@baulig baulig commented May 8, 2017

Move the call to SslStream.AuthenticateAsServer() into the constructor because
we don't want to call it again when Init() is called from Close().

Move the call to SslStream.AuthenticateAsServer() into the constructor because
we don't want to call it again when Init() is called from Close().
@davidsh
Copy link
Contributor

davidsh commented May 8, 2017

Where are the new tests to validate this change? This code is not just used on Mono so it needs to be generally applicable.

@baulig
Copy link
Author

baulig commented May 8, 2017

It's an old bug that has been in Mono's version for ages, before CoreFx was based on that codebase. It just recently came up when a customer filed a bug report with a rather complicated project, but I was able to track it down to making multiple requests on the same connection.

Adding tests - while surely desirable - would be a little bit complicated because it requires HttpListener with SSL, which is currently not supported in CoreFx on non-Windows platforms due to this FIXME:
https://github.com/dotnet/corefx/blob/master/src/System.Net.HttpListener/src/System/Net/Managed/HttpEndPointListener.cs#L58

@@ -103,6 +103,9 @@ public HttpConnection(Socket sock, HttpEndPointListener epl, bool secure, X509Ce
}

_timer = new Timer(OnTimeout, null, Timeout.Infinite, Timeout.Infinite);
if (_sslStream != null) {
_sslStream.AuthenticateAsServer (_cert, true, (SslProtocols)ServicePointManager.SecurityProtocol, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting, allman braces, no spaces after function name..

Copy link
Member

Choose a reason for hiding this comment

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

Does .Net Framework's HttpListener use ServicePointManager.SecurityProtocol?

@davidsh davidsh removed their assignment May 8, 2017
@CIPop
Copy link
Member

CIPop commented May 8, 2017

Adding tests - while surely desirable - would be a little bit complicated because it requires HttpListener with SSL, which is currently not supported in CoreFx on non-Windows platforms

Maybe it would be better to postpone this and add it as a dependent task to #14691 to ensure we're not regressing anything?

@@ -103,6 +103,9 @@ public HttpConnection(Socket sock, HttpEndPointListener epl, bool secure, X509Ce
}

_timer = new Timer(OnTimeout, null, Timeout.Infinite, Timeout.Infinite);
if (_sslStream != null) {
_sslStream.AuthenticateAsServer (_cert, true, (SslProtocols)ServicePointManager.SecurityProtocol, false);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: this can now be:

_sslStream?.AuthenticateAsServer(_cert, true, (SslProtocols)ServicePointManager.SecurityProtocol, false);

@akoeplinger
Copy link
Member

@CIPop we'd prefer merging this now instead of waiting for #14691 so we can move forward integrating the code in Mono :)

@stephentoub stephentoub merged commit b9608bd into dotnet:master May 11, 2017
@baulig baulig deleted the work-httplistener-52508 branch May 11, 2017 20:22
@karelz karelz modified the milestone: 2.0.0 May 13, 2017
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.

8 participants