Implement straightforward ServicePoint(Manager) properties in HttpWebRequest by liveans · Pull Request #94664 · dotnet/runtime

liveans · 2023-11-13T13:20:48Z

I'm going to create an issue with a list of properties that are not implemented.
I'm still working on the AllowWriteStreamBuffering property, I'll create a follow-up PR for this.

Implemented properties from ServicePoint:

TcpKeepAlive
ReceiveBufferSize
ContinueTimeout
Expect100Continue

ghost · 2023-11-13T13:21:02Z

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

Issue Details

I'm going to create an issue with a list of properties that are not implemented.
I'm still working on the AllowWriteStreamBuffering property, I'll create a follow-up PR for this.

Implemented properties from ServicePoint:

TcpKeepAlive
UseNagleAlgorithm
ReceiveBufferSize

Implemented properties from ServicePointManager:

ReusePort

Author:	liveans
Assignees:	-
Labels:	`area-System.Net`
Milestone:	-

liveans · 2023-11-13T13:21:21Z

/azp list

azure-pipelines · 2023-11-13T13:21:27Z

CI/CD Pipelines for this repository: runtime-coreclr outerloop runtime-coreclr jitstress runtime-coreclr jitstressregs runtime-coreclr jitstress2-jitstressregs runtime-coreclr gcstress0x3-gcstress0xc runtime-coreclr gcstress-extra runtime-coreclr r2r-extra runtime-coreclr jitstress-isas-x86 runtime-coreclr jitstress-isas-arm runtime-coreclr jitstressregs-x86 runtime-coreclr libraries-jitstressregs runtime-coreclr libraries-jitstress2-jitstressregs runtime-coreclr r2r runtime-coreclr runincontext runtime-coreclr crossgen2 runtime-libraries-coreclr outerloop runtime-libraries-coreclr outerloop-windows runtime-libraries-coreclr outerloop-linux runtime-libraries-coreclr outerloop-osx runtime runtime-libraries enterprise-linux runtime-libraries stress-http runtime-libraries stress-ssl runtime-dev-innerloop runtime-coreclr crossgen2 outerloop coreclr-release-outerloop-nightly runtime-coreclr crossgen2-composite runtime-jit-experimental runtime-coreclr libraries-jitstress dotnet-linker-tests runtime-coreclr ilasm runtime-coreclr crossgen2-composite gcstress runtime-staging runtime-coreclr pgo runtime-coreclr libraries-pgo Antigen runtime-community Fuzzlyn runtime-coreclr superpmi-replay runtime-wasm runtime-coreclr superpmi-diffs runtime-coreclr superpmi-asmdiffs-checked-release runtime-extra-platforms jit-cfg runtime-wasm-perf runtime-llvm runtime-coreclr jitstress-random runtime-coreclr libraries-jitstress-random runtime-android-grpc-client-tests runtime-wasm-libtests runtime-android runtime-androidemulator runtime-ioslike runtime-ioslikesimulator runtime-linuxbionic runtime-maccatalyst runtime-coreclr pgostress runtime-coreclr jitstress-isas-avx512 runtime-tools-tests runtime-libraries-mono outerloop runtime-sanitized runtime-wasm-dbgtests runtime-wasm-optional runtime-nativeaot-outerloop

liveans · 2023-11-13T13:21:43Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-11-13T13:21:53Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov

I assume it's impossible to implement functional tests for this. How do you test/validate the changes?

liveans · 2023-11-14T07:03:07Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-11-14T07:03:29Z

Azure Pipelines successfully started running 1 pipeline(s).

liveans · 2023-11-14T07:16:42Z

I assume it's impossible to implement functional tests for this. How do you test/validate the changes?

I'm testing these options manually, but I'll try to write some tests for this.

wfurt

generally looks good to me.
We should perhaps add at least some get/set tests so we exercise the new code path.

ManickaP · 2023-11-17T13:06:36Z

What issue(s) does this fixes/contributes to? Could you put them to the description.

ManickaP

+1 for the tests as well, some coverage would be nice.

liveans · 2023-11-20T14:07:22Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-11-20T14:07:32Z

Azure Pipelines successfully started running 1 pipeline(s).

liveans · 2023-12-04T13:18:57Z

/azp run runtime

azure-pipelines · 2023-12-04T13:19:31Z

Azure Pipelines successfully started running 1 pipeline(s).

liveans · 2023-12-04T14:35:52Z

@dotnet/ncl This is ready for final review, I've tested implemented features through Wireshark, and it's working.

liveans · 2023-12-04T15:49:54Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-12-04T15:50:08Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov

Product code LGTM, a few suggestions for the tests.

Co-authored-by: Anton Firszov <antonfir@gmail.com>

liveans · 2023-12-04T16:48:15Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-12-04T16:48:25Z

Azure Pipelines successfully started running 1 pipeline(s).

wfurt

LGTM % comments.
Good improvement.

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>

liveans · 2023-12-08T12:58:16Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-12-08T12:58:27Z

Azure Pipelines successfully started running 1 pipeline(s).

liveans · 2023-12-10T10:19:05Z

Outerloop failures unrelated

Implement TcpKeepAlive and some properties from ServicePoint

251d5e6

liveans requested a review from a team November 13, 2023 13:20

ghost added the area-System.Net label Nov 13, 2023

ghost assigned liveans Nov 13, 2023

antonfirsov reviewed Nov 13, 2023

View reviewed changes

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

Delete unicast set socket option

85f3c78

build-analysis Bot mentioned this pull request Nov 14, 2023

Intermittend failures in ThreadPoolTests on Mono Linux ARM64 #85960

Closed

wfurt reviewed Nov 16, 2023

View reviewed changes

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

wfurt reviewed Nov 16, 2023

View reviewed changes

ManickaP reviewed Nov 17, 2023

View reviewed changes

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

stephentoub reviewed Nov 18, 2023

View reviewed changes

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

stephentoub reviewed Nov 18, 2023

View reviewed changes

Comment thread src/libraries/System.Net.Requests/src/System/Net/ServicePoint/ServicePointManager.cs

Ahmet Ibrahim Aksoy (from Dev Box) added 5 commits November 20, 2023 13:32

Review feedback

df765c6

Delete else case

ef8b4a3

Make TcpKeepAlive reference type

09bf6c4

Delete unnecessary usings

b280083

Compile error fix

2762e90

MihaZupan reviewed Nov 22, 2023

View reviewed changes

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

Review Feedback

7ff1ce6

Correct test cases naming

1132f7c

MihaZupan reviewed Nov 30, 2023

View reviewed changes

build-analysis Bot mentioned this pull request Nov 30, 2023

[mono][tvos] OOM in System.IO.Tests.MemoryStreamTests #92467

Closed

Ahmet Ibrahim Aksoy (from Dev Box) added 2 commits November 30, 2023 20:32

Review feedback

796ab94

Add Expect 100 Continue header tests

9eaf5c6

antonfirsov reviewed Dec 4, 2023

View reviewed changes

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

build-analysis Bot mentioned this pull request Dec 4, 2023

[wasm] JSImportExportTest.JsImportTaskTypes - System.ObjectDisposedException #93347

Closed

Ahmet Ibrahim Aksoy and others added 2 commits December 4, 2023 17:12

Apply suggestions from code review

9b2148c

Co-authored-by: Anton Firszov <antonfir@gmail.com>

Review feedback

b25c529

antonfirsov approved these changes Dec 4, 2023

View reviewed changes

This was referenced Dec 4, 2023

Test failure WindowsAlternateDataStreamOverwrite #83659

Closed

Test failure: Expect100Continue_WaitsExpectedPeriodOfTimeBeforeSendingContent #58628

Closed

MihaZupan approved these changes Dec 6, 2023

View reviewed changes

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

wfurt approved these changes Dec 6, 2023

View reviewed changes

Ahmet Ibrahim Aksoy and others added 2 commits December 8, 2023 12:57

Apply suggestions from code review

42bbd8c

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>

Review feedback

6663b4f

liveans merged commit f0a6dbd into dotnet:main Dec 10, 2023

github-actions Bot locked and limited conversation to collaborators Jan 10, 2024

liveans deleted the httpwebrequest-straightforward-missing-parts-impl branch January 15, 2024 10:59

karelz added this to the 9.0.0 milestone May 14, 2024

Uh oh!

Conversation

liveans commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ghost commented Nov 13, 2023

liveans commented Nov 13, 2023

azure-pipelines Bot commented Nov 13, 2023

liveans commented Nov 13, 2023

azure-pipelines Bot commented Nov 13, 2023

antonfirsov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liveans commented Nov 14, 2023

azure-pipelines Bot commented Nov 14, 2023

liveans commented Nov 14, 2023

Uh oh!

wfurt left a comment

Choose a reason for hiding this comment

ManickaP commented Nov 17, 2023

ManickaP left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

liveans commented Nov 20, 2023

azure-pipelines Bot commented Nov 20, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liveans commented Dec 4, 2023

azure-pipelines Bot commented Dec 4, 2023

liveans commented Dec 4, 2023

liveans commented Dec 4, 2023

azure-pipelines Bot commented Dec 4, 2023

antonfirsov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

liveans commented Dec 4, 2023

azure-pipelines Bot commented Dec 4, 2023

Uh oh!

Uh oh!

Uh oh!

wfurt left a comment

Choose a reason for hiding this comment

liveans commented Dec 8, 2023

azure-pipelines Bot commented Dec 8, 2023

liveans commented Dec 10, 2023

Labels

7 participants

liveans commented Nov 13, 2023 •

edited

Loading