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.

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

Implement Header encoding selectors on SocketsHttpHandler #39468

Merged
merged 15 commits into from
Jul 30, 2020
Merged

Implement Header encoding selectors on SocketsHttpHandler #39468

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
22ca8bb
Expose HeaderEncodingSelectors on SocketsHttpHandler
MihaZupan Jul 13, 2020
2c425e4
Implement header encoding selectors in SocketsHttpHandler
MihaZupan Jul 15, 2020
17b43ec
Add header encoding tests
MihaZupan Jul 16, 2020
43b8caf
Add summaries for new APIs
MihaZupan Jul 16, 2020
5a13ba0
Use Stream.Write(byte[], int, int) overloads for framework compat
MihaZupan Jul 19, 2020
202268f
Add dummy API implementation to browser target
MihaZupan Jul 19, 2020
6c6a078
Merge branch 'master' into header-encoding-socketshttp
MihaZupan Jul 22, 2020
5aeaf40
HPack/QPack fixes
MihaZupan Jul 22, 2020
7a44c28
Move HeaderEncodingSelector delegate to namespace, add TContext
MihaZupan Jul 22, 2020
be0dcdd
Encoding fixes
MihaZupan Jul 23, 2020
c171683
Remove unused using
MihaZupan Jul 27, 2020
9b4fb5d
Merge master
MihaZupan Jul 28, 2020
23c1d61
Simplify test
MihaZupan Jul 28, 2020
5cc1ddd
HttpConnection PR feedback
MihaZupan Jul 28, 2020
4e248e4
Simplify fast-path detection
MihaZupan Jul 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Expose HeaderEncodingSelectors on SocketsHttpHandler
  • Loading branch information
@MihaZupan
MihaZupan committed Jul 14, 2020
commit 22ca8bbec057e4a1feb12d4276517ad74b4e5cc6
3 changes: 3 additions & 0 deletions src/libraries/System.Net.Http/ref/System.Net.Http.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,17 @@ public SocketsHttpHandler() { }
public bool PreAuthenticate { get { throw null; } set { } }
public System.Collections.Generic.IDictionary<string, object?> Properties { get { throw null; } }
public System.Net.IWebProxy? Proxy { get { throw null; } set { } }
public System.Net.Http.SocketsHttpHandler.HeaderEncodingSelector? RequestHeaderEncodingSelector { get { throw null; } set { } }
public System.TimeSpan ResponseDrainTimeout { get { throw null; } set { } }
public System.Net.Http.SocketsHttpHandler.HeaderEncodingSelector? ResponseHeaderEncodingSelector { get { throw null; } set { } }
[System.Diagnostics.CodeAnalysis.AllowNullAttribute]
public System.Net.Security.SslClientAuthenticationOptions SslOptions { get { throw null; } set { } }
public bool UseCookies { get { throw null; } set { } }
public bool UseProxy { get { throw null; } set { } }
protected override void Dispose(bool disposing) { }
protected internal override System.Net.Http.HttpResponseMessage Send(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { throw null; }
protected internal override System.Threading.Tasks.Task<System.Net.Http.HttpResponseMessage> SendAsync(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { throw null; }
public delegate System.Text.Encoding? HeaderEncodingSelector(string headerName, System.Net.Http.HttpRequestMessage request);
Copy link
Member

@halter73 halter73 Jul 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@Tratcher What do you think about switching Kestrel's RequestHeaderEncodingSelector to be a delegate System.Text.Encoding? HeaderEncodingSelector(string headerName) delegate?

It would be a new delegate type most likely defined in the Microsoft.AspNetCore.Server.Kestrel namespace since we don't have an HttpRequestMessage or really anything comparable in the early stages of request parsing. Having the same name as the System.Net.Http delegate might be too confusing though.

I originally went with Func<string, Encoding?> (that's at least what it will be once we support nullable reference types in that assembly) because we don't use delegates very much in configuration callbacks in ASP.NET Core. Thinking about it more though, that's because normally the argument is a specific options or context type. Since the argument is just a string, naming it feels more worthwhile. Increased consistency with System.Net.Http is an added bonus.

Copy link
Member

Choose a reason for hiding this comment

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

It would be a small usability improvement.

I'd avoid using the same name though, that gets awkward when you import both namespaces.

}
public partial class StreamContent : System.Net.Http.HttpContent
{
Expand Down
7 changes: 6 additions & 1 deletion ...ibraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ internal sealed class HttpConnectionSettings
internal TimeSpan _expect100ContinueTimeout = HttpHandlerDefaults.DefaultExpect100ContinueTimeout;
internal TimeSpan _connectTimeout = HttpHandlerDefaults.DefaultConnectTimeout;

internal SocketsHttpHandler.HeaderEncodingSelector? _requestHeaderEncodingSelector;
internal SocketsHttpHandler.HeaderEncodingSelector? _responseHeaderEncodingSelector;

internal Version _maxHttpVersion;

internal bool _allowUnencryptedHttp2;
Expand Down Expand Up @@ -101,7 +104,9 @@ public HttpConnectionSettings CloneAndNormalize()
_useCookies = _useCookies,
_useProxy = _useProxy,
_allowUnencryptedHttp2 = _allowUnencryptedHttp2,
_assumePrenegotiatedHttp3ForTesting = _assumePrenegotiatedHttp3ForTesting
_assumePrenegotiatedHttp3ForTesting = _assumePrenegotiatedHttp3ForTesting,
_requestHeaderEncodingSelector = _requestHeaderEncodingSelector,
_responseHeaderEncodingSelector = _responseHeaderEncodingSelector
};
}

Expand Down
23 changes: 23 additions & 0 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading;
using System.Threading.Tasks;
using System.Diagnostics.CodeAnalysis;
using System.Text;

namespace System.Net.Http
{
Expand Down Expand Up @@ -276,6 +277,28 @@ public TimeSpan Expect100ContinueTimeout
public IDictionary<string, object?> Properties =>
_settings._properties ?? (_settings._properties = new Dictionary<string, object?>());

public delegate Encoding? HeaderEncodingSelector(string headerName, HttpRequestMessage request);
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved

public HeaderEncodingSelector? RequestHeaderEncodingSelector
{
get => _settings._requestHeaderEncodingSelector;
set
{
CheckDisposedOrStarted();
_settings._requestHeaderEncodingSelector = value;
}
}

public HeaderEncodingSelector? ResponseHeaderEncodingSelector
{
get => _settings._responseHeaderEncodingSelector;
set
{
CheckDisposedOrStarted();
_settings._responseHeaderEncodingSelector = value;
}
}

protected override void Dispose(bool disposing)
{
if (disposing && !_disposed)
Expand Down