Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixes bug that prevents generation of new captcha iden for failed calls to VCaptcha-decorated API methods #730

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

buddydvd commented Apr 4, 2013

How to reproduce the issue (one of the many ways, see bottom for more):

  1. Create a new reddit account (or login to one that still see/needs CAPTCHA)
  2. Click "Submit a new text post" (don't worry, we won't actually submit anything)
  3. For title, enter "test"
  4. For subreddit, enter a non-existent subreddit name (e.g. "asfasdfasdfasdfsadf")
  5. Enter the correct solution for the CAPTCHA challenge
  6. Click "submit"

Expected result: The submission fails with the subreddit field decorated with the "that subreddit doesn't exist" error message AND the CAPTCHA image refreshes.

Actual result: The submission fails with the subreddit field decorated with the "that subreddit doesn't exist" error message BUT the CAPTCHA image doesn't refresh.

Summary of this change: Update validator.py to issue new CAPTCHA identifier any time when calls to a VCaptcha-decorated API method fail.

Additional notes: The following API methods (along with their corresponding web forms) are affected by this bug:

Contributor

bboe commented Apr 4, 2013

👍

@bsimpson63 bsimpson63 commented on an outdated diff Apr 10, 2013

r2/r2/lib/validator/validator.py
@@ -276,8 +276,8 @@ def _validatedForm(self, self_method, responder, simple_vals, param_vals,
# add data to the output on some errors
for validator in simple_vals:
- if (isinstance(validator, VCaptcha) and
- form.has_errors('captcha', errors.BAD_CAPTCHA)):
+ if (isinstance(validator, VCaptcha) and c.errors.errors):
@bsimpson63

bsimpson63 Apr 10, 2013

Owner

Might be more clear to do something like:

if (isinstance(validator, VCaptcha) and
    (form.has_errors('captcha', errors.BAD_CAPTCHA) or c.errors.errors)):
    form.new_captcha()

Also, could just use c.errors in place of c.errors.errors because ErrorSet defines __len__

Contributor

buddydvd commented Apr 10, 2013

Okay, updated. To preserve your diff comment, I avoiding using rebase to update this pull request. Instead, I reverted the previous commit and created a new commit with your suggested changes. So the last commit is ready to be reviewed and (hopefully) cherry picked.

💇

Owner

bsimpson63 commented Apr 11, 2013

🐟 looks good

@bsimpson63 bsimpson63 closed this Apr 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment