Skip to content

Tweaks to the new form csrf caching entry #4772

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

Merged
merged 2 commits into from
Jan 16, 2015
Merged

Tweaks to the new form csrf caching entry #4772

merged 2 commits into from
Jan 16, 2015

Conversation

weaverryan
Copy link
Member

Q A
Doc fix? no
New docs? no
Applies to all
Fixed tickets n/a

Hi guys!

After merging this nice new entry in #4141, I wanted to make a few minor language tweaks (the diff looks bigger than the changes really are). This included shortening a few sections and talking less about how all reverse proxies refuse to cache pages with a session, because I don't (for example) believe this is true with Symfony's reverse proxy.

Thanks!

@weaverryan weaverryan changed the title [#4141] Tweaks to the new form csrf caching entry Tweaks to the new form csrf caching entry Jan 4, 2015
@dbu
Copy link
Contributor

dbu commented Jan 5, 2015

seems good to me, easier to read and no mistakes introduced as far as i can see.

Typically, each user is assigned a unique CSRF token, which is stored in
the session for validation. This means that if you *do* cache a page with
a form containing a CSRF token, you'll cache the CSRF token of the *first*
user only. When a user submits, the token won't match the token stored in
Copy link
Member

Choose a reason for hiding this comment

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

I think there are some missing words here:

When a user submits, the token ...

Proposal:

When a user submits the form, the token ...

@weaverryan weaverryan merged commit cc40b5c into 2.3 Jan 16, 2015
weaverryan added a commit that referenced this pull request Jan 16, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Tweaks to the new form csrf caching entry

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | no
| Applies to    | all
| Fixed tickets | n/a

Hi guys!

After merging this nice new entry in #4141, I wanted to make a few minor language tweaks (the diff looks bigger than the changes really are). This included shortening a few sections and talking less about how *all* reverse proxies refuse to cache pages with a session, because I don't (for example) believe this is true with Symfony's reverse proxy.

Thanks!

Commits
-------

cc40b5c Adding missing words thanks to javiereguiluz
1c568e1 [#4141] Tweaks to the new form csrf caching entry
@xabbuh xabbuh deleted the csrf-form-tweaks branch January 16, 2015 17:58
for each user.

How to Cache Most of the Page and still Be Able to Use CSRF Protection
Why Caching Pages with a CSRF token are Problematic
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be "is" instead of "are"? And should we add a label for the old headline?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right! Fixed at sha: 36d1bac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants