Skip to content

HttpFoundation - uncomment test assertion #17150

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

Closed
wants to merge 1 commit into from
Closed

Conversation

keradus
Copy link
Member

@keradus keradus commented Dec 27, 2015

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

@TomasVotruba
Copy link
Contributor

👍

@Tobion
Copy link
Contributor

Tobion commented Dec 27, 2015

This test seems wrong according to

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

When this status code is returned for a byte-range request, the response SHOULD include a Content-Range entity-header field specifying the current length of the selected resource (see section 14.16). This response MUST NOT use the multipart/byteranges content- type.

@keradus
Copy link
Member Author

keradus commented Dec 28, 2015

So since assertion works the implementation does not follow RFC ?

@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

@keradus Indeed. Can you work on a fix?

@keradus
Copy link
Member Author

keradus commented Mar 3, 2016

Sorry @fabpot, I didn't get that deep into RFC.

fabpot added a commit that referenced this pull request Mar 10, 2016
…sted Range is unsatisfied (jakzal)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpFoundation] Set the Content-Range header if the requested Range is unsatisfied

| Q             | A
| ------------- | ---
| Branch        | 2.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This is a followup to #17150 (comment)

[RFC2616](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) specifies the Content-Range header SHOULD be included with a *416 Requested Range Not Satisfiable* response:

>    When this status code is returned for a byte-range request, the response SHOULD include a Content-Range entity-header field specifying the current length of the selected resource (see section 14.16). This response MUST NOT use the multipart/byteranges content- type.

[RFC 7233](https://tools.ietf.org/html/rfc7233#section-4.2) specifies what should be the header's value. It's in the "Request for comments" state, but it's the best definition I could find. This value is valid according to rfc2616 as well.

Commits
-------

54329d8 [HttpFoundation] Set the Content-Range header if the requested Range is unsatisfied
symfony-splitter pushed a commit to symfony/http-foundation that referenced this pull request Mar 10, 2016
…sted Range is unsatisfied (jakzal)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpFoundation] Set the Content-Range header if the requested Range is unsatisfied

| Q             | A
| ------------- | ---
| Branch        | 2.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This is a followup to symfony/symfony#17150 (comment)

[RFC2616](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) specifies the Content-Range header SHOULD be included with a *416 Requested Range Not Satisfiable* response:

>    When this status code is returned for a byte-range request, the response SHOULD include a Content-Range entity-header field specifying the current length of the selected resource (see section 14.16). This response MUST NOT use the multipart/byteranges content- type.

[RFC 7233](https://tools.ietf.org/html/rfc7233#section-4.2) specifies what should be the header's value. It's in the "Request for comments" state, but it's the best definition I could find. This value is valid according to rfc2616 as well.

Commits
-------

54329d8 [HttpFoundation] Set the Content-Range header if the requested Range is unsatisfied
@keradus keradus deleted the 2.3-x branch February 9, 2025 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants