Dollar signs followed by a number in the values of replacement arguments passed to DrupalWebTestCase::buildXPathQuery() seem to be handled as a regular expression back reference, causing certain values to be changed inappropriately.

$query = $this->buildXPathQuery(':test', array(':test' => '$1bc'));

I expected $query to have the value: "$1bc"
Actual value however is: "bc"

Use case: we use DrupalWebTestCase::randomString() quite often to generate node titles. DrupalWebTestCase::randomString() might generate something that is recognized as a back reference.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pfrenssen’s picture

Version: 7.x-dev » 8.x-dev
Status: Active » Needs review
Issue tags: +Needs backport to D7
FileSize
1.14 KB
2.47 KB

This problem is also present in D8, let's fix it there first.

Status: Needs review » Needs work

The last submitted patch, 1988780-1-simpletest-backreferences-test_only.patch, failed testing.

pfrenssen’s picture

Status: Needs work » Needs review
Cyberwolf’s picture

Are backreferences starting with double slash handled as well (for example: \\1)? That also seems to be an issue.

pfrenssen’s picture

Good catch, supported them in this patch, and followed your advice about using preg_replace_callback(). Also added a test to cover the double slash backreferences.

angel.h’s picture

I've made the 7.x patch based on the patch 5 from the previous comment.

angel.h’s picture

Version: 7.x-dev » 8.x-dev
Cyberwolf’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, thanks!

alexpott’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review

Committed 43b389d and pushed to 8.x. Thanks!

Now #6 needs review for 7.x

Damien Tournoud’s picture

Status: Needs review » Needs work

Needs work for Drupal 7, as anonymous functions are only supported on PHP 5.3+ and Drupal 7 supports 5.2+.

angel.h’s picture

Assigned: Unassigned » angel.h
angel.h’s picture

Assigned: angel.h » Unassigned
angel.h’s picture

Issue summary: View changes

small correction

pfrenssen’s picture

It's not possible to do this with preg_replace_callback() in PHP 5.2. You can only create anonymous functions using create_function() but this does not allow to pass variables from the current scope, like you can do with closures in PHP5.3.

Let's try it with the venerable str_replace() instead.

pfrenssen’s picture

Forgot to upload the test_only patch. This one should fail.

Status: Needs review » Needs work

The last submitted patch, 14: 1988780-13-simpletest-backreferences-test_only.patch, failed testing.

The last submitted patch, 13: 1988780-13-simpletest-backreferences.patch, failed testing.

pfrenssen’s picture

Tests fail because str_replace() does not respect the word boundary as preg_replace('//\b/') does, it also replaces :date in "xsd:dateTime" below:

$tracker_activity = $this->xpath('//tr[@about=:url]//td[contains(@property, "dc:modified") and contains(@property, "sioc:last_activity_date") and contains(@datatype, "xsd:dateTime") and @content=:date]', array(':url' => $url, ':date' => $isoDate));

We have to respect these word boundaries.

  • alexpott committed 50ab0be on 8.3.x
    Issue #1988780 by pfrenssen: Fixed DrupalWebTestCase::buildXPathQuery()...

  • alexpott committed 50ab0be on 8.3.x
    Issue #1988780 by pfrenssen: Fixed DrupalWebTestCase::buildXPathQuery()...

  • alexpott committed 50ab0be on 8.4.x
    Issue #1988780 by pfrenssen: Fixed DrupalWebTestCase::buildXPathQuery()...

  • alexpott committed 50ab0be on 8.4.x
    Issue #1988780 by pfrenssen: Fixed DrupalWebTestCase::buildXPathQuery()...