Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpFoundation] SendContent-Length when callingResponse::send() and the content is a non-empty string#45092

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:6.1fromnicolas-grekas:response-length
Apr 12, 2022

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedJan 20, 2022•
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?yes
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

More hints for the client and the webserver are always good.

As discussed on Slack with@bastnic

@bastnic
Copy link
Contributor

To add some context: I've a problem somewhere in my stack that truncate my big json response (2.5mo).

I expected Symfony to add the headerContent-Length that would help but it seems not. Anybody knows a good reason not to ?

@stof
Copy link
Member

Thisprepare method is called on the kernel.response event at priority 0, while the WebProfilerBundle will inject the toolbar later. So this would then report thewrong content-length header, which seems worse to me. MaybeResponse::setContent should update theContent-Length header if it is present.

@stof
Copy link
Member

And very early versions of Symfony used to put that header, which has been removed to fix an issue with gzip:#1846

@nicolas-grekas
Copy link
MemberAuthor

Thanks for link@stof, PR updated to update the header insetContent() instead.

@nicolas-grekasnicolas-grekasforce-pushed theresponse-length branch 2 times, most recently fromfd0de27 to2d59966CompareJanuary 20, 2022 14:58
@nicolas-grekasnicolas-grekas changed the title[HttpFoundation] send Content-Length when content is a non-empty string[HttpFoundation] send Content-Length when content is a stringJan 20, 2022
@stof
Copy link
Member

Given the potential impact (the old ticket also mentions that adding theContent-Length prevents Apache to use chunked transfer encoding), should this be merged as a bugfix or a new feature ?

@stof
Copy link
Member

Also, the PR description looks wrong. It is not fixing the bug from 2011 (which was already fixed by removing the content-length header at that time)

@nicolas-grekasnicolas-grekas changed the title[HttpFoundation] send Content-Length when content is a string[HttpFoundation] send Content-Length when content is a non-empty stringJan 20, 2022
@nicolas-grekasnicolas-grekas changed the base branch from4.4 to6.1January 20, 2022 15:12
@nicolas-grekasnicolas-grekas modified the milestones:4.4,6.1Jan 20, 2022
@nicolas-grekas
Copy link
MemberAuthor

OK to target 6.1
PR updated. Setting the header insetContent() breaks HttpCache.
Now doing it insend()

@nicolas-grekas
Copy link
MemberAuthor

Review welcome @symfony/mergers

@nicolas-grekasnicolas-grekas changed the title[HttpFoundation] send Content-Length when content is a non-empty string[HttpFoundation] SendContent-Length when callingResponse::send() and the content is a non-empty stringApr 11, 2022
@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commitaff969d intosymfony:6.1Apr 12, 2022
@fabpotfabpot deleted the response-length branchApril 12, 2022 06:00
@fabpotfabpot mentioned this pull requestApr 15, 2022
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull requestMay 31, 2022
…when calling `Response::send()` and the content is a non-empty string (nicolas-grekas)"This reverts commitaff969d, reversingchanges made to8b680f0.
nicolas-grekas added a commit that referenced this pull requestMay 31, 2022
…g `Response::send()` and the content is a non-empty string" (nicolas-grekas)This PR was merged into the 6.1 branch.Discussion----------[HttpFoundation] Revert "Send `Content-Length` when calling `Response::send()` and the content is a non-empty string"| Q             | A| ------------- | ---| Branch?       | 6.1| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#46449| License       | MIT| Doc PR        | -Let's revert#45092 as it's breaking BC. It's not worth it.Commits-------7e24e5d Revert "feature#45092 [HttpFoundation] Send `Content-Length` when calling `Response::send()` and the content is a non-empty string (nicolas-grekas)"
nicolas-grekas added a commit that referenced this pull requestMay 31, 2022
* 6.1:  [Workflow] Add ZEturf as backer  [Serializer] code cleanup  [Serializer] Forget partially collected traces  Added missing __call to TraceableEncoder  Revert "feature#45092 [HttpFoundation] Send `Content-Length` when calling `Response::send()` and the content is a non-empty string (nicolas-grekas)"  [PropertyInfo] Fix extracting int range type  [FrameworkBundle] Add alximy as backer of version 6.1  [WebProfilerBundle] Fix AJAX requests info are truncated in the WDT
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoyceruto approved these changes

@chalasrchalasrchalasr approved these changes

+2 more reviewers

@94noni94noni94noni left review comments

@TobionTobionTobion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

9 participants

@nicolas-grekas@bastnic@stof@fabpot@Tobion@94noni@yceruto@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp