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.

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

Conditional in HttpConnection.cs seems redundant? #94261

Closed
omajid opened this issue Nov 1, 2023 · 3 comments · Fixed by #94278
Closed

Conditional in HttpConnection.cs seems redundant? #94261

omajid opened this issue Nov 1, 2023 · 3 comments · Fixed by #94278

Comments

@omajid
Copy link
Member

omajid commented Nov 1, 2023

if (_chunked && _context.Response.ForceCloseChunked == false)
{
// Don't close. Keep working.
_reuses++;
Unbind();
Init();
BeginReadRequest();
return;
}
_reuses++;
Unbind();
Init();
BeginReadRequest();
return;

The same code is executed if the conditional is true or false. Is the conditional supposed to do something different?

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 1, 2023
@ghost
Copy link

ghost commented Nov 1, 2023

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

Issue Details

if (_chunked && _context.Response.ForceCloseChunked == false)
{
// Don't close. Keep working.
_reuses++;
Unbind();
Init();
BeginReadRequest();
return;
}
_reuses++;
Unbind();
Init();
BeginReadRequest();
return;

The same code is executed if the conditional is true or false. Is the conditional supposed to do something different?

Author: omajid
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@stephentoub
Copy link
Member

This code originally came from Mono, and history in this repo has those two blocks always being the same; looking in mono/mono, it goes back upwards of 15 years, and arrived like this after multiple refactorings. I'm not sure what the original intent was, but we can delete the duplication now.

@CarnaViire
Copy link
Member

Would you like to submit a PR @omajid?

From triage perspective, putting it to Future. We would welcome the contribution. Thanks!

@CarnaViire CarnaViire added this to the Future milestone Nov 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 1, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 1, 2023
omajid added a commit to omajid/dotnet-runtime that referenced this issue Nov 3, 2023
The code does the same thing whether the conditional is true or false,
so remove the conditional.

Also remove the now-unused `_chuncked` field.

Fixes: dotnet#94261
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 4, 2023
stephentoub pushed a commit that referenced this issue Nov 4, 2023
The code does the same thing whether the conditional is true or false,
so remove the conditional.

Also remove the now-unused `_chuncked` field.

Fixes: #94261
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2023
@karelz karelz modified the milestones: Future, 9.0.0 May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants