Allow only HTTP requests during X509 chain building #48221
Conversation
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks |
| object? socketsHttpHandler = socketsHttpHandlerCtor.Invoke(null); | ||
| pooledConnectionIdleTimeoutProp.SetValue(socketsHttpHandler, TimeSpan.FromSeconds(PooledConnectionIdleTimeoutSeconds)); | ||
| object? httpClient = Activator.CreateInstance(httpClientType, new object?[] { socketsHttpHandler }); | ||
| allowAutoRedirectProp.SetValue(socketsHttpHandler, false); |
There was a problem hiding this comment.
I think it'd be worth including a todo comment citing that we should clean up this method if #45364 is ever implemented.
|
|
||
| // Get the relevant types needed. | ||
| Type? socketsHttpHandlerType = Type.GetType("System.Net.Http.SocketsHttpHandler, System.Net.Http, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", throwOnError: false); | ||
| Type? httpMessageHandlerType = Type.GetType("System.Net.Http.HttpMessageHandler, System.Net.Http, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", throwOnError: false); |
There was a problem hiding this comment.
This method is getting pretty complicated with all this reflection. As a follow-up, I wonder if it'd be worthwhile adding a single internal method to SocketsHttpHandler that does all this logic without reflection (and without duplication of code for getting a redirect Uri), and then just reflect to invoke that one member from here.
There was a problem hiding this comment.
Apparently part of me still believes this is Core 1.0, because my initial thought was "but what about package shear?". Since we're both inbox I guess that's safe enough 😄
| // }; | ||
| // var httpClient = new HttpClient(socketsHttpHandler); | ||
| object? socketsHttpHandler = Activator.CreateInstance(socketsHttpHandlerType); | ||
| // Note: using a ConstructorInfo instead of Activator.CreateInstance, so the ILLinker can see the usage through the lambda method. |
There was a problem hiding this comment.
I don't think this comment makes sense. This isn't being called inside the lambda method. It should be fine to continue calling Activator.CreateInstance here.
| pooledConnectionIdleTimeoutProp.SetValue(socketsHttpHandler, TimeSpan.FromSeconds(PooledConnectionIdleTimeoutSeconds)); | ||
| object? httpClient = Activator.CreateInstance(httpClientType, new object?[] { socketsHttpHandler }); | ||
| allowAutoRedirectProp.SetValue(socketsHttpHandler, false); | ||
| object? httpClient = httpClientCtor.Invoke(new object?[] { socketsHttpHandler }); |
There was a problem hiding this comment.
Why did this change from Activator.CreateInstance?
ghost
locked as resolved and limited conversation to collaborators
4 participants
Fixes #45010
This is a port of 5.0 change. There was a merge conflict with fdc8148 which I have resolved here.
Adding @eerhardt to validate this will not produce new ILLink warnings.
This is a fix related to dotnet/announcements#175
cc: @karelz @Anipik