Different load arguments for multiple % wildcards in path. [#367343]

If I create the following menu item in hook_menu, then calling that path causes the fatal error PHP Fatal error: Exception thrown without a stack frame in Unknown on line 0 in D7 (A page not found in D6).


  $items['test/%node/%test'] = array(
    'title' => 'Test',
    'load arguments' => array('%map', '%index'),
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

This happens because node_load does not check the $revision argument that gets passed in for validity as a vid. i.e. all vid's should be numeric. I'm not sure how this might affect the node_load_multiple, but for node_load the following patches should correct the issue. At least they do for me.

Comment	File	Size	Author
#8	multiple-load-arguments-367343-8.patch	1.23 KB	cdale
#8
#2	node-load-arguments-367343-2-D7.patch	1.12 KB	cdale
#2
	node-load-arguments-D6.patch	771 bytes	cdale

	node-load-arguments-D7.patch	1.09 KB	cdale

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 February 2009 at 01:15

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #2

cdale CreditAttribution: cdale commented 1 February 2009 at 09:23

Status:

Needs work

» Needs review

File	Size
node-load-arguments-367343-2-D7.patch	1.12 KB

Reroll ro remove notices.

Comment #3

cdale CreditAttribution: cdale commented 1 February 2009 at 09:24

Priority:

Normal

» Critical

I feel this is probably critical. Feel free to correct me.

Comment #4

9 February 2009 at 05:50

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #5

cdale CreditAttribution: cdale commented 9 February 2009 at 21:44

Status:

Needs work

» Needs review

Comment #6

cdale CreditAttribution: cdale commented 20 February 2009 at 22:50

Title:	Fatal Error when %node used in menu with 'load arguments'	» Can't use 'load arguments' with %node. Fatal error.
Component:	node system	» menu system

Does anyone else have any designs, ideas or other ways to achieve using load arguments with %node? Is this a flaw with the menu system that this can not be done? Or is the fault with node_load?

Comment #7

Damien Tournoud CreditAttribution: Damien Tournoud commented 20 February 2009 at 22:52

Status:

Needs review

» Closed (won't fix)

  $items['test/%node/%test'] = array(
    'title' => 'Test',
    'load arguments' => array('%map', '%index'),
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

This should be:

  $items['test/%node/%'] = array(
    'title' => 'Test',
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

Comment #8

cdale CreditAttribution: cdale commented 21 February 2009 at 02:52

Title:	Can't use 'load arguments' with %node. Fatal error.	» Different load arguments for multiple % wildcards in path.
Category:	bug	» feature
Priority:	Critical	» Normal
Status:	Closed (won't fix)	» Needs review

File	Size
multiple-load-arguments-367343-8.patch	1.23 KB

Is there some documentation for this somewhere that I've missed? Does this only apply when using 'load arguments'? I guess this would not be a problem if the 'load arguments' key was not there.

I think it could be useful to be able to do this, as such I've made a patch.

It allows load arguments to be defined as below:

  $items['test/%wild/path/%test'] = array(
    'title' => 'Test',
    'load arguments 1' => array('1', 'TEST'),
    'load arguments 3' => array('%map', '%index'),
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

This allows for multiple _load functions to be used, with different load arguments for each if desired. It will also work as is currently i.e the code below will still work:

  $items['test/%test/path/%'] = array(
    'title' => 'Test',
    'load arguments' => array('%map', '%index'),
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

Does this seem like a reasonable solution? What are peoples thoughts for allowing this? Probably not a huge demand for it, but it could be useful, and it is a minimal change to allow it, and it breaks no existing functionality that I can see.

Comment #9

cburschka

they

CreditAttribution: cburschka commented 26 April 2009 at 10:24

Status:

Needs review

» Needs work

    'load arguments 1' => array('1', 'TEST'),
    'load arguments 3' => array('%map', '%index'),

I'm not well-versed enough in the menu system to know whether this feature is needed, but this property should definitely be an array rather than numbered keys:

  $items['test/%wild/path/%test'] = array(
    'title' => 'Test',
    'load arguments' => array(
      1 => array('1', 'TEST'),
      3 => array('%map', '%index'),
    ),
    'description' => "Test some code here",
    'page callback' => 'test_call',
    'page arguments' => array(1, 2),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

This also allows for a convenient is_array() check (making it backward compatible). What you lose is the option of defining the same load arguments for several placeholders, but not all. But that seems like a rare edge case where you may as well define the load arguments for each placeholder separately.