Simply put, DrupalMatchPathTest sets the front page to a randomName() string, and then matches (among other things) that string against the path pattern "".

If the first character in the front page path is a number, drupal_match_path() will remove that character while generating a regular expression. The result is here:

// Front page is wGbP.
/^(wGbP)$/	--	wGbP -- true

// Front page is "2SHH"
/^(SHH)$/	--	2SHH -- false

// Front page is "y6Os"
/^(y6Os)$/	--	y6Os -- true

Note that this you cannot reproduce this manually by creating a path alias, because the path alias will be resolved before being matched. The test bypasses this by setting a front page path that doesn't actually exist.

The reason can probably be found in the regular expression:

'\1' . preg_quote(variable_get('site_frontpage', 'node'), '/') . '\2'

You can see that a leading numerical digit will become part of the back reference index.

This is a very weird edge case, true. I caught it only because I had to hack the db_prefix out of randomName() to get the file field tests to work (see #667416: Filefield tests fail with db_prefix > 6 characters), and even then it only results in 16% of cases. But I tested again with a clean checkout and a prefix of "7_" (which is plausible) and it breaks as well.

CommentFileSizeAuthor
#3 drupal_match_path-backref-668672-3.patch992 bytescburschka
#2 drupal_match_path-backref-668672-2.patch992 bytescburschka

Comments

cburschka’s picture

cburschka
they
commented
Priority: Critical » Normal

Update: I thought it was critical before I realized that the code base normally prefixes the randomName() with a db_prefix, turning this into an edge case rather than a random failure.

cburschka’s picture

cburschka
they
commented
Status: Active » Needs review
StatusFileSize
new drupal_match_path-backref-668672-2.patch992 bytes

The preg_replace() documentation has actually foreseen this edge case, and provides a guideline on how to handle it:

When working with a replacement pattern where a backreference is immediately followed by another number (i.e.: placing a literal number immediately after a matched pattern), you cannot use the familiar \\1 notation for your backreference. \\11, for example, would confuse preg_replace() since it does not know whether you want the \\1 backreference followed by a literal 1, or the \\11 backreference followed by nothing. In this case the solution is to use \${1}1. This creates an isolated $1 backreference, leaving the 1 as a literal.

Here's a patch.

cburschka’s picture

cburschka
they
commented
StatusFileSize
new drupal_match_path-backref-668672-3.patch992 bytes

The PHP documentation there appears to be incorrect; the pattern that must be used is actually ${1}, not \${1} (at least in single-quote). Here is a patch that passes the test.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Needs work

Yep, nice catch. But:

(1) let's use ${2} for the second one too
(2) add a code comment
(3) add an explicit test for a path starting with a number

int’s picture

int commented

see the #535440: Random strings in SimpleTest should not contain $db_prefix

int’s picture

int commented

The testbot work well without this fix.
So I think that this should be fixed but we should only point [TESTING BROKEN] if breaks the testbot.

cburschka’s picture

cburschka
they
commented
Title: [TESTING BROKEN] DrupalMatchPathTest fails when db_prefix starts with a number. » DrupalMatchPathTest fails when db_prefix starts with a number.

You are of course right; the test is broken but the testbot is not. This is not a critical issue for getting our patch reviews back.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.