| Project: | Address Field |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Problem/Motivation
I tried this on a completely fresh install of the latest Drupal 7.17 and Addressfield 7.x-1.0-beta3.
- Enable addressfield.
- Add an address field instance to the user entity. Require its inclusion on the user registration form and require the user supply their address.
- Enable drupal page cache and set the minimum cache lifetime to something like an hour.
- Clear your page cache
- Visit the user registration page at /user/register. This should be a cache miss.
- Address field should default to the US. Select any US state (like "Washington").
- Change the country to Brazil.
- Navigate to the user registration page again. This should be a Drupal cache hit.
- Address field should default to the US. Select any US state again (like "Washington").
- Change the country to Italy.
You should now see something like the following image where an "illegal choice has been detected" message is shown and the form shown represents the Brazilian address field variation (with Brazilian states), even though Italy is selected as the country.
The net result is that when page caching is enabled, users are able to negatively affect the user experience for independent and completely unrelated users who subsequently attempt to submit the form.
This is especially aggravated if you're automatically selecting state/country values on the frontend.
Seems like addressfield shouldn't cache form and form state values when a user selects a different country.
| Attachment | Size |
|---|---|
| Screen shot 2012-12-07 at 4.45.07 PM.png | 47.2 KB |
Comments
#1
After extensive investigation, it would appear as though there may be something unexpected with how core suppresses validation errors when #limit_validation_errors is set. I don't know the full scope of the issue.
I've confirmed that Address Field is using #limit_validation_errors correctly, and form.inc ultimately calls
form_set_error(NULL, '', $form_state['triggering_element']['#limit_validation_errors']);as expected, but even still, the validation error isn't adequately suppressed.We can hack around it in address field with this patch. This may be a terrible idea, but it works around the problem.
For a slew of related, but ultimately unhelpful reading:
#2
To make this even more obvious and even simpler to reproduce, I've provided a test. In addition to more basic AJAX tests, it tests the exact scenario described in the issue summary above.
I've attached two patches: a test-only patch (which should fail when run), and a patch with the fix from #1.
I've also renamed the issue to be more clear. I'm also going to bump this to major as per this page describing major as an "error which is only triggered under rare circumstances or which affects only a small percentage of all users"
#3
With proper patches this time...
#4
I can confirm this bug, and it's more critical if you try to populate the form with country and state/province information that is different than the default set.
I.E. if the default is the U.S. and I try to populate the country with Canada and the province of British Columbia, the form will break.
#5
Why is this form getting cached? That seems odd to me... how can a user with a different session hit the form as rendered by another session to begin with?
#6
This is related to the Drupal form API's misuse of cache backends as semi-persistent storage for form structure.
The key used to access the form data (stored in cache_form) is based on the validation token associated with the form, which was generated when the form was initially rendered. As such, the validation token for a given form on a given page will be identical for all anonymous users when the Drupal page cache is enabled.
Initially, this doesn't matter because they see the form rendered as it was originally rendered (with all defaults / no data filled out).
When you change the country on an Address field, for what I believe are reasons related to form validation (don't quote me there), it actually changes (and saves in cache_form) an alternate structure for the form relevant to the country. The difference most relevant here is on administrative area where it can change from a textfield to a select list (or from one select list to another). Once the form is actually submitted (as opposed to AJAX-submitted), it attempts to validate the user's input against a now-invalid form structure.
Again, because cache_form's keys are based on the validation token of the originally rendered page, and because all anonymous users have access to that form/token/page, all any anonymous users have the ability to mess with the structure against which actual form POSTs will be validated.
#7
Note that even with the patch above, we were still seeing this error come through on occasion. We ended up going even further:
<?php
// If country change triggered the submission, kill all errors and messages.
if (_form_element_triggered_scripted_submission($element, $form_state)) {
drupal_get_messages();
drupal_static_reset('form_set_error');
// If the country was changed via AJAX, and the user is anonymous, the
// existing page cache entry will be invalid, so we have to wipe it. Note
// we're using the referer here because we're in an AJAX context.
if (isset($_SERVER['HTTP_REFERER']) && $GLOBALS['user']->uid == 0) {
cache_clear_all($_SERVER['HTTP_REFERER'], 'cache_page');
}
}
?>
It's a big hammer to swing, and even still we see one or two slip through each day (presumably in situations where users are filling out a given form simultaneously).