| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | forms system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | needs backport to D7, Needs issue summary update |
Issue Summary
Problem/Motivation
Login to any Drupal site fails if the following circumstances come together:
- core caching feature for anonymous users switched on
- login form being submitted via Ajax
- more than one user wants to login since FAPI has created and stored the form in cache
How to reproduce:
- adjust the settings in Drupal: enable caching and make the login form being submitted via Ajax
- Open the page http://www.example.com/user from two separate browsers
- Reload those pages as often as you like, you can verify in html source of the pages in both browsers, that the form-build-id has the same value
- Now login to the site from one of those browsers. That should work fine.
- After that (!), login from the other browser without reloading the page before doing that. This login attempt will fail. You will fine a record in watchdog that says 'Invalid form POST data'.
Another check, if you turn off Ajax submission for that form, the exact same scenario will not cause that problem.
Proposed resolution
The behaviour is down to ajax_get_form() which deletes the cached record of the form straight after loading it from cache. This is why subsequent calls to the same function with the same form-build-id will then fail as the record is no longer available in cache.
The function drupal_get_form() which is used without Ajax is not deleting the record from cache after having loaded it and that's why the behaviour is different.
To resolve this problem I can see two options:
- Either do cache forms always individually, so that each delivered instance of a form has its own form-build-id
- Or do not delete the form from cache when going through ajax_get_form()
But I'm not sure which one is the "correct" one as I can't oversee possible side effects at this point.
Remaining tasks
For now, someone who understands this better needs to decide what's the best approach.
User interface changes
None.
API changes
Don't think any would arise from fixing this.
Original report by jurgenhaas
When Drupal's core caching feature is switched on, forms are delivered to anonymous visitors from cache which results in the situation where one and the same form gets delivered more than once with the same form_build_id.
As soon as one of the anonymous users submits such a form, the record gets removed from cache.
The next visitor who is submitting the same form will potentially run into trouble because of the missing form record in cache.
The regular Drupal FAPI seems to be OK with that but if the form gets submitted via Ajax, the submission gets rejected, a watchdog entry gets issued ('Invalid form POST data') and drupal_exit() gets called.
The result of that is that the user would have to reload the page before being able to submit the form. This can be annoying i.e. if the form being used is the user login form. It seems as if just nothing is happening.
This issue arrises always when page caching for anonymous users and form submission via Drupal's own ajax mechanism get used in combination.
Original Post:
Enabling 'cache pages for anonymous users' results in the following behavior :
Login as user 'foo'. Login successful.
Login as another user 'bar'. Results in "Invalid form POST data'.
logout as user'foo'. try to re-login. Results in "Invalid form POST data'.
Disabling caching allows normal login.
I am a newbie to drupal, so it is entirely possible i have done something wrong. In that case pl. close the issue with my apologies.
In the firebug,the unsuccessful login attempts record a "200 OK" response.
Also clearing the cache in the admin menu, allows for one more login after which the issue persists again.
(P.S: In trying to reproduce this in the demo page, i noticed a minor issue: if an user name is already taken, one is not able to change the user name and login in the same form).
Thanks,
-kvh
Comments
#1
Just came across the very same problem and looked around on how I could fix it. Nothing in ajax_register turned out to be the culprit for this so I tried it on a site without ajax_register installed and found the very same problem on the user login page. That looks to me like a Drupal core bug.
#2
Now loocked a bit deeper into this and this is most certainly a Drupal core issue in relation to forms being submitted through ajax.
Here is what happens:
When page caching is turned on and if we grab the form for user login, we always get a form for each anonymous user with the same form_build_id. This can be reproduced when you use two different browsers on the same PC and call the same page www.example.com/user. You can now have a look into the html source and will see that both forms come with the same form_build_id.
If both users from both browsers are using that form to login, this seems to be working fine - which puzzels me, but that's a separate issue.
However, if the form was configured to be submitted through ajax, then the second one (and all subsequent ones) will fail. This is because the form will be deleted from cache as soon as it has been submitted and then the subsequent submission from user 2 will be rejected in ajax_get_form() because the form can't be found in cache anymore.
Consequently I ask myself two questions:
1) Should any form ever be delivered twice with the same form_build_id? I guess not.
2) How can we avoid delivery of the login form twice with the same form_build_id?
Looking for advise, this is really serious I guess.
#3
Reassigning the issue as I believe this is Drupal core.
#4
Can you write an issue summary? It's not clear how this is a core bug.
#5
Just updated the Issue summary
#6
Any feedback on this one? It is causing real pain!
#7
Same problem here.
Two identical drupal installations, ajax enabled login form, site A no caching, site B caching, on site B when i log-in->log-out i can't login back, the ajax page is called but no response, just watchdog error popping.
Help...
#8
I'm also encountering this issue, not fun.
#9
I think that this problem deserve a critical categorization.
#10
This still needs a clear issue summary.
#11
Ok I'll try to be clearer.
Drupal 7 installation with an AJAX enabled login form.
Starting situation (just cleared the cache).
Login form_build_id "form-sqXHwJympdAcZGTGIeHaZgsWGBfcE1ubCVH_-m1CsDo"
Form working as expected, ajax request sent e reply received.
Then I logout, refresh the login page.
Login form_build_id "form-sqXHwJympdAcZGTGIeHaZgsWGBfcE1ubCVH_-m1CsDo" (the same as before?)
when i click on login an ajax request call /system/ajax but an empty response is given.
I put some breakpoints in validation and submit functions, but the code didn't pass here. The problem is earlier.
I guess that the issue is related to the fact that the form_build_id doesn't change even if I refresh the page if I enable caching on admin performance page.
If I disable the caching, everything works as expected, the form_build_id change every page refresh.
I tried to put the $form_state['cache'] = FALSE or $form_state['no_cache'] = TRUE key but it seem not to work.
#12
To be more specific, everything works if I disable "cache pages for anonymous users"
#13
The login form is in a block, on the navigation region, so it's on every page.
On the cache form table sometimes I find the cached form, that disappear on the first submit, and never pop again.
Sometimes instead the cached form is still there after the submit, but the valid token is always the same (so the validation check fails).
The problem is that on form submission Drupal run the ajax_get_form function (http://api.drupal.org/api/drupal/includes!ajax.inc/function/ajax_get_form/7) sometimes he can't find the cached forms, sometimes he found it but the form_get_cache fails on valid token check (http://api.drupal.org/api/drupal/includes!form.inc/function/form_get_cache/7)
if ((isset($form['#cache_token']) && drupal_valid_token($form['#cache_token'])) || (!isset($form['#cache_token']) && !$user->uid))#14
Help still needed :(
#15
I've just written an issue summary according to the template and hope that tim.plunkett can now take this forward.
#16
That's not possible with a stock Drupal 7 install. Please either post steps to reproduce from a clean install of Drupal 7, or indicate the contrib or custom code that's contributing to this.
#17
I've created a block, available in top region on all pages:
function mymodule_block_info(){
$blocks['mymodule_login'] = array(
'info' => t('My User Login'),
'status' => FALSE,
'weight' => 0,
'cache' => DRUPAL_NO_CACHE,
);
return $blocks;
}
function mymodule_block_view($delta){
switch($delta){
case 'mymodule_login':
$block['content'] = mymodule_block_userlogin();
return $block;
break;
}
}
function mymodule_block_userlogin(){
return drupal_get_form('user_login_block');
}
Then i've altered the login form
function mymodule_form_alter(&$form, &$form_state, $form_id){
switch($form_id){
case 'user_login_block':
$form["#validate"][2]="mymodule_user_login_final_validate";
$form['actions']['submitlogin'] = $form['actions']['submit'];
unset($form['actions']['submit']);
$form['login_error'] = array(
'#type' => 'markup',
'#prefix' => '<div id="login_error">',
'#suffix' => '</div>',
'#markup' => '',
);
$form['name']['#attributes']['onkeypress'][]='if(event.keyCode==13){jQuery(\'#edit-submitlogin\').trigger(\'trigLogin\');}';
$form['pass']['#attributes']['onkeypress'][]='if(event.keyCode==13){jQuery(\'#edit-submitlogin\').trigger(\'trigLogin\');}';
$form['actions']['submitlogin']['#ajax'] = array(
'callback' => 'mymodule_form_login_callback',
'event' => 'trigLogin',
);
$form['actions']['submitlogin']['#attributes']['onkeypress'][]='if(event.keyCode==13){jQuery(\'#edit-submitlogin\').trigger(\'trigLogin\');}';
$form['actions']['submitlogin']['#attributes']['onmousedown'][]='jQuery(\'#edit-submitlogin\').trigger(\'trigLogin\');';
break;
}
}
When I deactivate the page caching everything works, but if I activate page chaching the form works only once as said before.
Neither the validate neither the callback functions are called, the workflow exits earlier (https://drupal.org/node/1694574#comment-6546788)
#18
#19
This is not criticial unless there is prove that this is a core bug and not caused by your custom code or a contrib module. Please to not set it back to critical without confirming that.
#20
Well, I can't really agree to your assesment here, @Berdir. The form API is from Drupal core and the Ajax framework is from Drupal core too. And both behave fairly different with regard to form submission and they cause severe issues as described in the issue summary: a form with the same form_build_id is delievered many times and as soon as it is submitted the first time, this makes all the other delivered form unusable.
This is at least a major issue if not critical.
Agreed, core doesn't come with a way to submit the login form via Ajax but it provides all the credentials and they don't seem to be throught through properly. Yes, it only happens with some custom code or a contrib module but as clearly pointed out, the faulty behavior is in core.
So what else can we do to get this dealt with other then what we have done already? I'm happy to work on this, so I don't want to put more work on anyone'S shoulders that are carrying so much already. But I need assistance in final analysis and decision on how to resolve this.
#21
Yeah, as said by jurgenhaas, I want to collaborate to solve this issue, but all I can do is post the custom code I've created to submit via Ajax and as I said before, neither the form validation, nor the form callback are called, the problem pops before, on the form_get_cache core function ( http://drupal.org/node/1694574#comment-6546788 ).
I can't go forward than this...
Thank you for your work :)
#22
thank you to all the folks looking into this.
I have since moved to a non-AJAX login form, since caching is quite important for us.
-kvh
#23
I can't switch to a non-AJAX login, the customer do want a ajax response on the login form with a popup alert.... :(
#24
I've published the website for my customer without chaching enabled...I hope the server can carry the load...
#25
The reported bug sounds legit and sensible to me. Trying to clarify the issue title.
Attached patch implements the second solution proposal... but just now I realize that this does not fully solve problem, because:
The cache items will still be deleted when they are older than the expiration being set in
form_set_cache():// 6 hours cache life time for forms should be plenty.$expire = 21600;
Thus, even with this patch, the first user who tries to submit the form after 6 hours will still see the error message, since the form cache items were pruned / garbage collected, but the page containing the form (including the form_build_id) is still cached; perhaps even by a reverse-proxy.
That inherently means we have a much larger architectural problem with the current form cache.
However, this patch should at least fix the discrete problem that has been reported here - which apparently leads to much more frequent broken form submissions than 6 hours. Therefore, we might want to move forward with this independently.
Writing a test for this is going to be tough, since WebTestBase still lacks proper support for concurrent user sessions. (@see #1378126: Simplify tests needing multiple, concurrent user sessions / logins)
I'm also attaching a patch for D7, but only for facilitating tests of the proposed solution with your current code. Please do not attach any further D7 patches until this issue has been fixed for HEAD/D8 first.
#26
Closely related (and apparently conflicting): #343415: Form cache is not cleared on submit when page cache is activated
#27
I encountered what I thought was this bug... I was submitting a user login form via AJAX with caching turned on. The first time it was submitted was fine, but from that point on it failed, which sounds the same as described above. I tried applying the d7 patch from #25, but that didn't solve my problem so I dug deeper...
Turns out in my case the problem wasn't the cache getting cleared (which made sense due to the following line in around the caching):
<?phpif (!variable_get('cache', 0) && !empty($form_state['values']['form_build_id'])) { ... }
?>
It turns out in my case the problem was in
form_get_cache(), specifically the$form['#cache_token']was only valid for the first person to submit the form.I'm not sure if this is more of the same or if it's a completely separate issue. Would love to help resolve it, but not really sure where to start...
#28
Done a little more digging...
form_set_cache()has the following:<?phpif ($GLOBALS['user']->uid) {
$form['#cache_token'] = drupal_get_token();
}
?>
When the form is rebuilt after a login,
$GLOABLS['user']->uidcontains the freshly logged in user, meaning#cache_tokenis set.drupal_get_token()builds a token involvingsession_id()which then invalidates the form for any anonymous requests that follow.The only check
form_set_cache()uses for whether it sets a#cache_tokenis$GLOABLS['user']->uid... Perhaps we need to bring in an ability for a$form/$form_stateto stop it being set, which login forms make use of? I wonder if this is also an issue for registration forms.This only happens with AJAX requests, I've not quite figured out why that's the case though...
#29
Ok, so done a bit of playing. Here are patches that resolve the issue I was having with the user login via AJAX. I'm a bit unconvinced that this is actually the right way to resolve the bug, but it definitely confirms what the bug is...
In terms of tests, I don't think the problems with concurrency will be an issue. If caching is enabled, you can log in, log out and then try to log in again and that's when it'll fail. I'm not sure where the best place to put the tests are? I can't decide if the tests fall under ajax, cache, form or user... If you let me know what you think I'll write some tests for D8 and D7 which demonstrate the failure.
I'm also happy to update the attached patches for a better solution if you've got any pointers?
#30
Oops, that was supposed to be do-not-test... I've attached a module (dependent on ctools for the AJAX redirect command) which demonstrates the problem in Drupal 7... You need to turn anonymous page caching on.
#31
Sorry, forgot to attach...
#32
The last submitted patch, d8-1694574-29-preserver_cache_token_on_cached_ajax_submission.patch, failed testing.
#33
Apologies for posting up d7 patches rather than d8 but a client needed the d7 meaning I had to write them and I couldn't get d8 working locally to try and get it all working in d8. I will try again when I've got time, but in the mean time hopefully the patches can get some discussion as to whether this approach is the right approach at all... I didn't want to mess with the issue title/summary until someone who knows the Form API better than me confirms what I think is happening...
Possible solutions
The problem is in
form_set_cache()which, when there is a logged in user, sets#cache_tokenon the form. This then invalidates the form cache for any other requests, asajax_get_form()rightly tells the rebuild to copy the#build_id.I can see three possible solutions:
$form_stateto allow login submission handlers to preventform_set_cache()from storing the#cache_tokenin the rebuilt form.I've got a for the first in d7 along with a test which flags up the issue. I think I'm now as far as I can take this without having someone who knows Form API better than me giving some input.