Skip to content

Implement more ServicePoint properties#97537

Merged
liveans merged 41 commits into
dotnet:mainfrom
liveans:httpwebrequest-implement-more-property
Mar 1, 2024
Merged

Implement more ServicePoint properties#97537
liveans merged 41 commits into
dotnet:mainfrom
liveans:httpwebrequest-implement-more-property

Conversation

@liveans

@liveans liveans commented Jan 26, 2024

Copy link
Copy Markdown
Member

This PR implements:
HttpWebRequest:

  • DefaultMaximumErrorResponseLength

ServicePoint:

  • BindIPEndPointDelegate
  • ProtocolVersion
  • UseNagleAlgorithm
  • Certificate

ServicePoint but no effect (We're creating a new handler for every request if we have non-null ServicePoint):
These properties will matter in case we want to invest some work in caching of HttpClient under HttpWebRequest, the current implementation is straightforward and these properties are not gaining any advantage from it.

  • ConnectionLimit
  • MaxIdleTime
  • ConnectionLeaseTimeout

@ghost ghost added the area-System.Net label Jan 26, 2024
@ghost ghost assigned liveans Jan 26, 2024
@ghost

ghost commented Jan 26, 2024

Copy link
Copy Markdown

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

Issue Details

null

Author: liveans
Assignees: liveans
Labels:

area-System.Net

Milestone: -

@liveans

liveans commented Jan 26, 2024

Copy link
Copy Markdown
Member Author

Maybe I can implement the IdleSince property from ServicePoint by using PlaintextStreamFilter.

PlaintextStreamFilter = (context, ct) =>
{
    ServicePoint.IdleSince = DateTime.Now;

    return ValueTask.FromResult(context.PlaintextStream);
}

But not sure if this is a good idea, any suggestions @dotnet/ncl ?

Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebResponse.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebResponse.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebResponse.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebResponse.cs Outdated
@liveans liveans requested a review from MihaZupan February 27, 2024 07:22

@MihaZupan MihaZupan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a couple questions + style changes, otherwise this looks good

Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Outdated
Comment thread src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Outdated
Comment thread src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Outdated
Comment thread src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Outdated
Comment thread src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Outdated
@liveans

liveans commented Feb 28, 2024

Copy link
Copy Markdown
Member Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
@liveans

liveans commented Feb 28, 2024

Copy link
Copy Markdown
Member Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/libraries/System.Net.Requests/src/Resources/Strings.resx Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Outdated
Comment thread src/libraries/System.Net.Requests/src/System/Net/HttpWebResponse.cs Outdated
Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
@liveans

liveans commented Mar 1, 2024

Copy link
Copy Markdown
Member Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@liveans

liveans commented Mar 1, 2024

Copy link
Copy Markdown
Member Author

CI failures unrelated

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.

5 participants