8000 Improve specification explanation by matthieu88160 · Pull Request #7601 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

Improve specification explanation #7601

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 6 commits into from

Conversation

matthieu88160
Copy link
Contributor

RFC 2616 and RFC 7234 define that a recipient MUST ignore Expires header if one of s-max-age or max-age header is defined.

RFC 2616 and RFC 7234 define that a recipient MUST ignore Expires header if one of `s-max-age` or `max-age` header is defined.
.. _`expiration model`: http://tools.ietf.org/html/rfc2616#section-13.2
.. _`FrameworkExtraBundle documentation`: https://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/cache.html
.. _`RFC 7234 - Caching`: https://tools.ietf.org/html/rfc7234
Copy link
Member

Choose a reason for hiding this comment

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

I would make this a deep link to https://tools.ietf.org/html/rfc7234#section-5.3

Copy link
Member

Choose a reason for hiding this comment

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

Ah no, maybe that's not the best idea as https://tools.ietf.org/html/rfc7234#section-4.2.1 also contains information about the process that must be applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, the process is well explained in section 4.2.1

.. note::

Accordingly with `RFC 7234 - Caching`_, the ``Expires`` header value is
ignored when a ``s-max-age`` or ``max-age`` header is defined.
Copy link
Member

Choose a reason for hiding this comment

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

In fact, s-maxage (note the missing second dash) and max-age are not headers, but are directives of the Cache-Control header.

.. note::

Accordingly with `RFC 7234 - Caching`_, the ``Expires`` header value is
ignored when a ``s-max-age`` or ``max-age`` directives are defined.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should remove the a before directives.

.. note::

Accordingly with `RFC 7234 - Caching`_, the ``Expires`` header value is
ignored when ``s-max-age`` or ``max-age`` directives are defined.
Copy link
Member

Choose a reason for hiding this comment

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

[...] when the s-maxage or max-age directive of the Cache-Control header is defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xabbuh Minor comment, I don't get this change, it seems to me that both can be set at the same time, does it matter?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but it is enough for one of them to be present in order to ignore the Expires header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough then.

@xabbuh
Copy link
Member
xabbuh commented Apr 15, 2017

Thank you @matthieu88160.

xabbuh added a commit that referenced this pull request Apr 15, 2017
…guiluz)

This PR was submitted for the 3.2 branch but it was merged into the 2.7 branch instead (closes #7601).

Discussion
----------

Improve specification explanation

RFC 2616 and RFC 7234 define that a recipient MUST ignore Expires header if one of `s-max-age` or `max-age` header is defined.

Commits
-------

ce64af5 Update expiration.rst
46e6408 Update expiration.rst
b66e4d7 Update expiration.rst
13102da Update expiration.rst
8666bed Minor reword and fixed the line length
232c029 Improve specification explanation
xabbuh added a commit that referenced this pull request Apr 15, 2017
@xabbuh xabbuh closed this Apr 15, 2017
xabbuh added a commit that referenced this pull request Apr 15, 2017
* 2.7: (31 commits)
  Fixed the RST syntax
  Improve example context
  [#5621] Enhancing example of using bundle config
  [#7601] minor tweak
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Minor reword and fixed the line length
  Improve specification explanation
  [#7664] minor wording tweak
  Rewords and minor fixes
  Add an explanation about «constraints» validation
  [#7645] enumerate ordered list items implicitly
  Adding a new article about "Creating a Bug Reproducer"
  Fixed a syntax issue
  Use backticks
  #7311 choice_value callback argument can be null
  Fixed broken links for nginx & FastCgiExternalServer
  Update dialoghelper.rst
  ...
xabbuh added a commit that referenced this pull request Apr 15, 2017
* 2.8: (46 commits)
  [#7507] fix component name
  [#7490] minor typo fix
  Added a note about redirections to absolute URLs in tests
  Added the changes suggested by reviewers
  [#7620] use generate() in PHP templates before 2.8
  Fixed the RST syntax
  Improve example context
  [#5621] Enhancing example of using bundle config
  [#7601] minor tweak
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Minor reword and fixed the line length
  Improve specification explanation
  [#7664] minor wording tweak
  Rewords and minor fixes
  Add an explanation about «constraints» validation
  [#7645] enumerate ordered list items implicitly
  Adding a new article about "Creating a Bug Reproducer"
  ...
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.

5 participants
0