Problem/Motivation

Page caching does not seem to be disabled when using Honeypot on core Contact Forms
Steps to reproduce:

  1. Standard Drupal 8 install
  2. Download and Enable Honeypot (tested with HEAD)
  3. In Honeypot configuration, Enable protection on Website Feedback Contact Form and save configuration
  4. Visit /contact/feedback to cache page
  5. In another browser, while not logged in, visit /contact/feedback again and submit the form (Taking more than the default 5 seconds of course)
  6. You should incorrectly get "There was a problem with your form submission. Please wait X seconds and try again." message
  7. Visit /contact/feedback in additional browsers and note the honeypot timestamp has not changed

Proposed resolution

Remaining tasks

CommentFileSizeAuthor
#3 honeypot-contact-cache-2556233-3.patch3.14 KBjustachris
#3 honeypot-contact-cache-2556233-3-test-only.patch2.66 KBjustachris

Comments

justAChris created an issue. See original summary.

geerlingguy’s picture

geerlingguy commented

@justAChris - Thanks for the detailed report! I'll try to take a look soon. Hopefully we can add a test that confirms cache is enabled when timestamp protection is disabled, and vice-versa.

justachris’s picture

justachris commented
Status: Active » Needs review
StatusFileSize
new honeypot-contact-cache-2556233-3-test-only.patch2.66 KB
new honeypot-contact-cache-2556233-3.patch3.14 KB

Haven't tested across all the form applications, but this seems to work on the Contact form:
Drupal::service('page_cache_kill_switch')->trigger();

Added tests to look for cache hits in the header information.

Note that active core issue #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!) may also have some bearing on this as noted in the API change:

API changes
None. This is a pure addition; that doesn't change any APIs.

But, notable changes:
Forms are now marked as uncacheable (max-age = 0) by default.
...

The last submitted patch, 3: honeypot-contact-cache-2556233-3-test-only.patch, failed testing.

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Reviewed & tested by the community

Ha, that's exactly the change I was going to make in #2553319-5: disable page cache through the API rather than hacking global conf, but now you've gone and added a nice test and the change, so I'll merge this then backport the cache test to Drupal 7 so it works there as well.

And thanks for the heads up on SmartCache—I've been following along with interest, but didn't notice the note about forms being uncached by default!

Thanks!

geerlingguy’s picture

geerlingguy commented
Title: Caching not disabled on core Contact Form » Disable caching correctly on all forms
Related issues: +#2553319: disable page cache through the API rather than hacking global conf
geerlingguy’s picture

geerlingguy commented
Status: Reviewed & tested by the community » Fixed

Committed your patch.

geerlingguy’s picture

geerlingguy commented
Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Fixed » Patch (to be ported)

I'm going to backport the test to D7 (unless someone else gets to it earlier), and close out the other ticket. Thanks again!

justachris’s picture

justachris commented
Related issues: +#2525760: Sign timestamp values

Still had submission issues on contact forms, was still getting incorrectly getting "Please wait X seconds and try again" messages even after pulling latest. Narrowed it down to #2525760: Sign timestamp values, revised commit there should resolve the issue.

geerlingguy’s picture

geerlingguy commented
Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Patch (to be ported) » Fixed

Marking this as fixed in 8.x. For 7.x, the current method seems to have been working fine, so unless that changes I don't see any reason to change things.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.