Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Regular expression in drupalSetContent()
if (preg_match('/var drupalSettings = (.*?);/', $content, $matches)) {
$this->drupalSettings = drupal_json_decode($matches[1]);
}
assumes that there are no semicolons in Drupal.settings, but semicolon is a valid character.
There are not that many D8 modules in the wild (yet), but at least lightbox2 and google_analytics for D7 known to use semicolons in Drupal.settings. I'm sure there are other contrib modules as well.
Since $matches[1] contains only part of Drupal.settings, drupal_json_decode() returns false. This, for example, prevents testing of AJAX forms (drupal.Settings.ajax is empty, so drupalPostAjax() cannot determine triggering element's properties.
Attached patch fixes the problem
Comments
Comment #1
valthebaldComment #3
valthebaldEven simpler
Comment #4
valthebaldComment #5
miraje CreditAttribution: miraje commentedIndeed, the original regexp intends to take the whole value of Drupal.settings js var and put it into $matches[1]. It just doesn't takes into consideration that since a semicolumn will appear inside its value - the parsing won't be correct. In order to take the whole value of the Drupal.settings variable we need to make sure that we are taking the full string. So, the sign of line end should be added to regexp. The 2nd patch should do the job.
Comment #6
catchThis seems like it could have a test written for it.
Comment #7
valthebaldOops, seems I missed the last comment...
Anyway, here we go:
1. Test-only patch. I've added trivial setting to test-page (test-page-test module), and check it's value in HttpRequestTest.php. This should fail
2. Combined patch
Comment #8
valthebaldCorrect file names
Comment #9
miraje CreditAttribution: miraje commentedLast patch works the same as original (only change is test addition).
Comment #10
Dries CreditAttribution: Dries commentedLet's add a bit of documentation to the test please. Could be as short as one line/sentence.
Comment #11
valthebaldHere we go (I made comments line similar to "neighbour" comments)
Also, added other "suspicious" characters to the checked settings string.
Comment #12
miraje CreditAttribution: miraje commentedChecked again - all working, comment presents.
Comment #13
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #14
valthebaldSame patch, rerolled against 7.x
Comment #16
valthebaldMistakingly used drupal_http_request() instead of DrupalWebTestCase::drupalGet(), fixing
Comment #17
valthebaldCan this be applied to 7.x as well, please?
Comment #18
mgiffordNo longer applies.