Wildcard loader names cannot contain numbers. [#285165]

I am trying to define my own wildcard loader function and noticed that if I use a number as part of the wildcard loader name then it does not get called.

This below doesn't work. My function s3_object_load() does not get called, and nor does s3_object_page(). I end up with a 'page not found':

// in hook_menu:
$items['amazon/%s3_object/view'] = array(
    'title' => 'View S3 object',
    'page callback' => 's3_object_page',
    'page arguments' => array(1),
    'access callback' => 'user_access',
    'access arguments' => array('administer s3'), 
  );
}
// end of hook_menu snippet

// Loader function I want to use, but never gets called
function s3_object_load() {
  
}

The code below does work (the loader function gets called, and so does s3_object_page()):

// in hook_menu:
$items['amazon/%s_three_object/view'] = array(
    'title' => 'View S3 object',
    'page callback' => 's3_object_page',
    'page arguments' => array(1),
    'access callback' => 'user_access',
    'access arguments' => array('administer s3'), 
  );
}
// end of hook_menu snippet

// Loader function that works
function s_three_object_load() {
  
}

Note that the only change made was renaming the loader function from 's3_object' to 's_three_object'. I suspect there is a problem somewhere in the path parsing caused by the inclusion of digits in loader function names.

Any ideas?

Comment	File	Size	Author
#13	empty-or-valid-callback-285165-11.patch	923 bytes	pwolanin
#13
#12	285165-wildcard-fix.patch	879 bytes	Damien Tournoud
#12
#6	menu-inc-wildcard-loader-pattern.patch	966 bytes	Gábor Hojtsy
#6
#1	menu_object_loader.patch	735 bytes	Heine
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Heine CreditAttribution: Heine commented 21 July 2008 at 07:03

Status:

Active

» Needs review

File	Size
menu_object_loader.patch	735 bytes

You are correct. The replacement regular expression follows http://www.php.net/functions

Comment #2

Damien Tournoud CreditAttribution: Damien Tournoud commented 21 July 2008 at 07:43

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

Looks quite reasonable, but should go to 7.x first. The same patch should apply to 7.x (Heine, can you confirm?).

This is quite a small fix, so submitting "as is" for Dries review.

Comment #3

lilou CreditAttribution: lilou commented 23 August 2008 at 15:27

Patch applied successfully to CVS HEAD.

Comment #4

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 19 September 2008 at 03:43

Status:

Reviewed & tested by the community

» Needs work

Ha, that's a fun bug.

Can we please have a comment or two to explain what the hell that scary regex does? :)

Comment #5

Damien Tournoud CreditAttribution: Damien Tournoud commented 14 October 2008 at 12:38

The comment should cite http://php.net/manual/en/language.functions.php, which document that regexp.

Comment #6

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 October 2008 at 12:47

Status:

Needs work

» Reviewed & tested by the community

File	Size
menu-inc-wildcard-loader-pattern.patch	966 bytes

My issue on #321040: QUICKFIX: menu system does not allow numbers and uppercase in wildcard names was marked as a clear duplicate of this one. So let's get this fixed :) The attached patch includes comments and works for me with wildcards such as %l10n_community_language and %l10n_community_project.

Comment #7

Dries CreditAttribution: Dries commented 14 October 2008 at 13:32

Status:

Reviewed & tested by the community

» Fixed

Committed to CVS HEAD. Thanks for the documentation.

Comment #8

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 October 2008 at 13:38

Note that Dries also committed it to 6.x.

Comment #9

pwolanin CreditAttribution: pwolanin commented 14 October 2008 at 19:04

Priority:	Normal	» Critical
Status:	Fixed	» Active

this regex is wrong - it just broke a large fraction of all existing menu paths

that only matches if the % is followed by at least one char

Comment #10

pwolanin CreditAttribution: pwolanin commented 14 October 2008 at 19:10

here's one possible fix:

 if (preg_match('/^%$|^%([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)$/', $part, $matches)) {

Comment #11

chx CreditAttribution: chx commented 14 October 2008 at 19:17

%([a-zA-Z_][a-zA-Z_0-9]*)? do not overcomplicate. this plus _load is a PHP identifier. Starts with a letter or an underscore, continues the same plus numbers.