Submit with "return"/"enter" key in IE7 leads to failing CAPTCHA

advseb - July 29, 2009 - 07:50
Project:CAPTCHA
Version:6.x-2.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:postponed (maintainer needs more info)
Description

I only experience this problem with Internet Explorer 7. I enter the captcha solution. I confirm the solution pressing the return key. The captcha will always fail. I have to use the mouse to press the submit button of the node/comment edit form. Seems like a character is added by IE7 to the solution entered while pressing the return key.

#1

soxofaan - August 3, 2009 - 08:41
Component:Code» User interface
Category:bug report» support request

I don't have internet explorer, so I can't test/debug this.
But I've heard of problems with IE and submitting a form with enter before (e.g. see http://www.google.com/search?q=internet+explorer+enter+key+form, #194327: _form_builder_ie_cleanup not working -search submission)

In any case: it's technically not a bug in the CAPTCHA module as far as I know, but in Internet Explorer and it should be fixed/worked around in the Drupal Form API, I think. Which version of Drupal are you using?

Seems like a character is added by IE7 to the solution entered while pressing the return key.

can you confirm this? what character is added?

#2

advseb - August 4, 2009 - 08:10

I am using the latest Drupal 6 version. How can I find out which character is added to the input field?

#3

soxofaan - August 4, 2009 - 10:23

One way is enabling the logging of wrong answers (there is a checkbox on the CAPTCHA admin page for this) and looking at the watchdog log of your site. However I don't expect this to return reliable info.

A better way is hacking the CAPTCHA module and inserting code to inspect the answer directly, but I don't know if you are familiar with this.
I'll try to do it myself when I have access to a setup with internet explorer.

#4

advseb - August 4, 2009 - 10:56

I can change the module and add a watchdog entry. Just give me an idea where in the code I should add it.

#5

advseb - September 10, 2009 - 14:39
Version:6.x-1.0-rc2» 6.x-2.0-rc3
Category:support request» bug report

The problem still exists with RC3.

#6

advseb - September 10, 2009 - 14:42

I forgot to mention that I get the message "CAPTCHA test failed (unknown csid)" as described in this duplicate bug report: http://drupal.org/node/571886

#7

advseb - September 11, 2009 - 08:51
Version:6.x-2.0-rc3» 6.x-2.x-dev

I tried again with the current 2.x-dev release (2009-09-11) and the problem still exists. I get the following message in the watchdog list:

Message CAPTCHA validation error: unknown CAPTCHA session ID (NULL).
Severity error

The problem occurs with image_captcha and math_captcha.

#8

advseb - September 11, 2009 - 14:52

Here are some more debug outputs, which might give more insights what is going wrong. I added them to the function: function captcha_validate($element, &$form_state)

I added the following statements:

watchdog('debug', 'captcha_info: '.var_export($captcha_info, TRUE));
watchdog('debug', 'captcha_response: '.var_export($captcha_response, TRUE));
watchdog('debug', 'form_state: '.var_export($form_state['clicked_button']['#post']['captcha_sid'], TRUE));

In case I confirm in IE the form with RETURN key, I get the following output:

captcha_info: array ( 'form_id' => 'comment_form', 'module' => 'captcha', 'type' => 'Math', 'captcha_sid' => '44', 'solution' => '5', )
captcha_response: '6'
form_state: NULL <-----!!!

In case I confirm in IE the form by clicking on the publish button, I get the following output:

captcha_info: array ( 'form_id' => 'comment_form', 'module' => 'captcha', 'type' => 'Math', 'captcha_sid' => '44', 'solution' => '2', )
captcha_response: '5'
form_state: '44'

If I do the same again using Firefox 3.5, form_state is set correctly in both cases.

Question: Why don't you use 'captcha_sid' field from captcha_info? It seems to have the correct value in all cases?

Answer by myself: I tried using captcha_sid from captcha_info. In case I confirm the form using the button it works as expected. In case I hit RETURN, the same problem as before. It seems that at the moment the query looking for the solution in captcha_validate is issued, no entry exists if the form was submitted in IE using RETURN. The problem is that in captcha_process() function also the unset fields are used. Here, a solution might be something like:

$posted_form_id = (isset($form_state['clicked_button']['#post']['form_id']) ? $form_state['clicked_button']['#post']['form_id'] : $form_state['buttons']['submit'][0]['#post']['form_id']);
$captcha_sid = (isset($form_state['clicked_button']['#post']['captcha_sid']) ? $form_state['clicked_button']['#post']['captcha_sid'] : $form_state['buttons']['submit'][0]['#post']['captcha_sid']);

Please let me know if I should additional debug statements.

#9

soxofaan - September 11, 2009 - 21:33

at advseb in #8: very nice you're diving into the code to tackle this sucker. I don't have IE7. I'm on OSX and Linux and I only get up to IE6 with ie4linux and ie4osx. The problem is I can't reproduce this issue on IE6, is this issue IE7 specific?

Anyway, I'm a bit confused about your experiments in #8:
first you say that $captcha_info['captcha_sid'] is always available (even for IE+entersubmission), this value is set in captcha_process() and comes from around line 200 in captcha.module:

<?php
    $captcha_sid
= $form_state['clicked_button']['#post']['captcha_sid'];
?>

So, if it's available the above statement worked and the values were set.

But then you say

in captcha_process() function also the unset fields are used

So, could you look into this?

Another thing about your suggested solution is that $form_state['buttons'] does not always exist. E.g. for a CAPTCHA on the comment form, there is a $form_state['buttons'] , but for a CAPTCHA on a node form there is no $form_state['buttons'] , only $form_state['clicked_button']

thanks for looking into this

#10

advseb - September 12, 2009 - 18:06

I don't have access to the test systems on weekend, so no real answer possible before Monday. But I was wondering why it is so hard to embed those values in forms. I saw that you are using hidden fields. Aren't there easier ways to get those values? Have you looked, e.g., how the Mollom modules does it?

#11

advseb - September 14, 2009 - 11:30

Anyway, I'm a bit confused about your experiments in #8:
first you say that $captcha_info['captcha_sid'] is always available (even for IE+entersubmission), this value is set in captcha_process() and comes from around line 200 in captcha.module:

The $captcha_info structure is always set, because if you can't get a valid session id in captcha_process, you create a new one. The bug is really in captcha_process(). Probably, it will need a more finetuned way to get the form values in the different cases (node form, comment form, preview, etc.).

#12

soxofaan - September 14, 2009 - 13:26

But I was wondering why it is so hard to embed those values in forms.

The embedding is not hard. The difficulty is getting the right info back from the client/browser.

I saw that you are using hidden fields. Aren't there easier ways to get those values?

I'm open for suggestions.
In any case: it's best to keep the CAPTCHA solution on the server (only send the challenge to the client) and because HTTP is stateless, you need some sort of session concept to make CAPTCHA work.
In CAPTCHA branches 5.x-x.x and 6.x-1.x, we used the $_SESSION variable for this end, but this solution suffered from some problems (most importantly: cookies are required and the session table could grew very large because every anonymous visit was stored).
In CAPTCHA 6.x-2.x we do our own session housekeeping to avoid polluting the sessions table and do not require cookies. The CAPTCHA session ID is the key to pair challenges (send to the client) with solutions (kept at server). This CAPTCHA session ID is send with each CAPTCHA protected form as a hidden field.

Mollom uses the same principle with a hidden session ID field as far as I know.

There is are simpler keys to pair challenges with solutions, like the IP-address of the user to name something (which would not require a hidden field). But then you can only support (at most) one form per user at a time. So no support for multiple forms per page or multiple open forms per user in parallel (e.g. a user opening several post forms in separate tabs). And then we're still assuming that IP addresses are unique per user, which is not always true today.

Anyway, if you have suggestions: shoot

#13

rewe - September 15, 2009 - 14:11

hi, I'm a co-worker of advseb's -

here's what we did to fix this issue for now:

scenario: using the internet explorer and submitting a capture-enabled form by hitting the enter key rather than clicking submit or preview or whatever

symptom: the sub array ['clicked_button'] is not created which is why some checks in the functions captcha_process and captcha_validate fail and ultimately cause the overall capture solution to fail because a new csid is created every time the function captcha_process is called

solution: we added some logic that also checks the ['buttons'] sub array because it also has the required csid etc. in it

best regards

AttachmentSize
2009.09.15-captcha.module.patch 2.88 KB

#14

soxofaan - September 15, 2009 - 15:10
Status:active» needs work

Thanks for the patch.
I can't try it out for the moment however, so I just gave it a quick look.

I think this special case code should be refactored to a function, e.g. captcha_get_posted_captcha_sid($form_state) for better code reuse and to keep the pollution of the normal code low.

Did you already tried if it works with different forms (e.g. comment form, node form, user login form, user registration form, ...)?

#15

advseb - September 16, 2009 - 06:51

We are using it on the user registration form and the comment form. We have not tried the other forms.

#16

alisonjo2786 - September 24, 2009 - 15:56

Has anyone encountered this with the 5.x version?

#17

advseb - September 30, 2009 - 14:47

Is version 6-2.0 containing the fix?

#18

soxofaan - September 30, 2009 - 15:01

Is version 6-2.0 containing the fix?

No,
the current patch still has "needs work" status (see #14) and it is not confirmed it fixes it for all use cases (on comments, on node forms, on login forms, with extra AJAX in the mix, etc).

Getting a final 6.x-2.0 out of the door was long overdue and had a higher priority than this (non critical) issue.
Moreover, now that we have 6.x-2.0 out, we can get more eyes on this issue.

It should be fixed for CAPTCHA 6.x-2.1, though.

#19

advseb - October 1, 2009 - 12:52

Attached is a new version of the patch. It was created against 6.x-2.0 release branch. We also did the refactoring you suggested (moving code in an extra function). We are using the solution for comment and user registration forms and the patch is deployed on a large community already.

AttachmentSize
2009.10.01-captcha.module.patch 3.64 KB

#20

soxofaan - October 2, 2009 - 09:30

Thanks for the refactored patch,
I didn't have time yet to give it a closer look

but I wonder if the refactoring couldn't be pushed further, e.g. to put the stuff about $posted_form_id also in the refactored function

#21

advseb - October 5, 2009 - 08:33

I suggest to not refactor it in a function at the moment, because this function would only be used at one point in the code. There is a rule saying you should not refactor something until it is really needed.

#22

soxofaan - October 5, 2009 - 21:44
Component:User interface» Code
Status:needs work» needs review

I disagree,
refactoring is not only useful for fighting code duplication, it can also help make the code cleaner/better readable and maintainable.
Moreover, I think you can do the refactoring so it can be used in different places.
See attachment for what I mean.

AttachmentSize
534168_IEenter_03.patch 4.63 KB

#23

advseb - October 6, 2009 - 14:35

Ah ok, you are refactoring both things into one function. That is ok I think.

#24

soxofaan - November 9, 2009 - 15:03
Status:needs review» postponed (maintainer needs more info)

Hi,

I recently played a bit with IE7 to work on this issue, but I couldn't reproduce the problem.
I tried on comment form, user login block, user login page, forum post form, but all worked as expected when submitting with the enter key. (both the numpad enter key and the normal enter key)

Do you have a live site with the problem?

#25

Aren Cambre - November 11, 2009 - 14:16
Status:postponed (maintainer needs more info)» needs review

(Comment edited because it's not germane to this issue.)

If you enable CAPTCHA module and have caching enabled, anon users may not see CAPTCHA features on comment submission page and may get this error on comment review page: CAPTCHA validation error: unknown CAPTCHA session ID. Contact the site administrator if this problem persists.

Solution provided in next comment. It's a caching issue.

#26

soxofaan - November 11, 2009 - 10:12
Title:Confirming captcha with "return" key in IE7 leads to failed input» Submit with "return"/"enter" key in IE7 leads to failing CAPTCHA
Status:needs review» postponed (maintainer needs more info)

At Aren Cambre in #25: I'm afraid you are talking about a different issue here. This issue is specifically about posting a form in IE7 with the enter key instead of pressing the submit button with the mouse. You say you can "kind of duplicate" the issue, but you never mention using the enter key or submit button. So if it's not about enter key/submit button, please open your problem as a separate issue, to keep the discussion clean.

That being said, a possible cause of your problem is caching: if a page with form is cached before the CAPTCHA module is enabled and can do its thing, an anonymous user will get the form without CAPTCHA and posting will fail (with the error message you mentioned). I would suggest to clear (or temporarily disable) the page cache. If that does not work, please open a separate issue as I mentioned above.

#27

advseb - November 26, 2009 - 10:22

We have a live site with this problem, but we are using our fix now since several months and it works fine...

 
 

Drupal is a registered trademark of Dries Buytaert.