As it currently is, honeypot css file is generated:

- During install, defaulting to honeypot_element_name = 'url'
- When saving the honeypot setting in the admin form.

Now imagine a site where honeypot_element_name is deployed using features/strongarm, or there as an issue generating the original CSS file.

In both cases you are quite stuck with honeypot not working because the css file is missing/inappropriate, and this can be quite frustrating to diagnose specially if aggregation is turned on.

The module should probably verify (and regenerate if missing) the css file in honeypot_add_form_protection().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

david_garcia’s picture

Status: Active » Needs review
FileSize
1.35 KB

Not sure if this is the best approach, but does the job.

Status: Needs review » Needs work

The last submitted patch, 1: 2503309-ensure-css-is-ok.patch, failed testing.

david_garcia’s picture

Version: 7.x-1.17 » 7.x-1.x-dev
Status: Needs work » Needs review

geerlingguy’s picture

Status: Needs review » Needs work

I would rather regenerate the CSS file if it doesn't exist as part of the cron job, and not at runtime when the form is being built, as this will not impact per-request performance any further than it already is.

If we can move the logic from honeypot_add_form_protection() in this patch into honeypot_cron(), that would make this simpler and more performant, and worst case, a site would have a missing file for a few minutes (or a few hours at max) until cron runs.

chrisfree’s picture

I just ran into this issue and will see if I can get a patch created tonight/tomorrow.

chrisfree’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

@geerlingguy - I think this patch accomplishes what you are looking for.

  • geerlingguy committed 63da274 on 7.x-1.x authored by chrisfree
    Issue #2503309 by david_garcia, chrisfree: Honeypot does not ensure that...
geerlingguy’s picture

I committed the above patch, and am also going to commit the attached patch as a follow-up, which adds some tests around CSS file regeneration (that way we can ensure that cron-based regeneration will continue working, hopefully forever!

After I commit these extra changes, we'll need to forward-port the changes to Drupal 8 ASAP (unless the mechanism for building the file is radically different... it's been a few months since last time I worked on the D8 code).

geerlingguy’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Needs review » Patch (to be ported)

I've committed the above patch for D7. On to D8 next!

  • geerlingguy committed 9c760d7 on 7.x-1.x
    Issue #2503309 by geerlingguy: Honeypot does not ensure that honeypot....
geerlingguy’s picture

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

Nevermind... in Drupal 8, we add the CSS to the page's <head> dynamically, in a way that should work properly with things like modals. So no need to forward-port! And no need to backport to Drupal 6 since the CSS file generation was never backported.

Status: Fixed » Closed (fixed)

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