The recent security update introduces the following line of javascript in ajax.js which causes syntax error in IE8 (probably other browsers aswell).

./misc/ajax.js:625
$('input[name="form_build_id"][value="' + response.old + '"]').val(response.new);

The reason for this is of course the improper use of the reserved word 'new'.

Suggested change:
$('input[name="form_build_id"][value="' + response['old'] + '"]').val(response['new']);
I have changed the response.old aswell, just for consistency.

CommentFileSizeAuthor
#17 js-reserved-word-in-misc-ajax.js_.patch461 bytesbr0ken
#1 use-of-reserved-word-2245331-1.patch458 bytesbendikrb

Comments

bendikrb’s picture

bendikrb commented
StatusFileSize
new use-of-reserved-word-2245331-1.patch458 bytes

Patch is attached.

bendikrb’s picture

bendikrb commented
Status: Active » Needs review
Related issues: +#2243427: Update to Drupal 7.27 for SA-CORE-2014-002
David_Rothstein’s picture

David_Rothstein commented
Version: 7.27 » 7.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: +7.28 release blocker

Based on http://kangax.github.io/es5-compat-table/#Reserved_words_as_property_names this doesn't affect modern browsers (so in practice, it's mainly an issue for IE8 and earlier).

I tested the patch and it looks good and fixes the issue. Drupal 6 is not affected.

I also updated the "known issues" section of the Drupal 7.27 release notes and release announcements to point to this issue.

Thanks!

ahmad abbad’s picture

ahmad abbad commented

I tested the patch and it looks good and fixes the issue.

ParisLiakos’s picture

ParisLiakos commented

thanks, that was fast:)
related: #2034051-24: Error D.ajax is undefined

ngocketit’s picture

ngocketit commented

Tested and it works. Thanks for the patch!

alexander_danilenko’s picture

alexander_danilenko commented

Just found this bug and i'm too late, Issue already created. damn :)
I confirm that bug appears in IE8 and patch fixes it.

+1 to RTBC

scottatdrake’s picture

scottatdrake commented

Confirmed worked for me, too. (Dang you, IE8)

mike503’s picture

mike503 commented

+1 RTBC. This needs to get into 7.28. It's bad when a "security update" fix actually breaks things.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: use-of-reserved-word-2245331-1.patch, failed testing.

mlhess’s picture

mlhess commented
Status: Needs work » Needs review

1: use-of-reserved-word-2245331-1.patch queued for re-testing.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs review » Reviewed & tested by the community
sstedman’s picture

sstedman commented

+1 nice work, quick fix. Please include in 7.28. This broke js on my site and produced a very difficult to backtrace and fix error for IE8.

nod_’s picture

nod_
Primary language French
Location Lille
commented

Oh wow, sorry for not catching that during the review.

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -7.28 release blocker +7.28 release notes

Committed to 7.x - thanks!

  • Commit 5945c30 on 7.x by David_Rothstein: 
    Issue #2245331 by bendikrb: Use of reserved word in ajax.js breaks IE8.
br0ken’s picture

br0ken
Primary language Ukrainian
commented
StatusFileSize
new js-reserved-word-in-misc-ajax.js_.patch461 bytes

I think that the response['old'] is unnecessary because the "old" keyword isn't reserved for JavaScript language.

David_Rothstein’s picture

David_Rothstein commented

Right, it's unnecessary, but see the original issue report; the idea was to change both just to keep the style consistent in the same line of code.

Status: Fixed » Closed (fixed)

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