ZF-7771: Wrong action/id mapping in Zend_Rest_Route.

Issue Type:
Bug
Created:
2009-09-04T04:27:36.000+0000
Last Updated:
2009-09-06T19:03:58.000+0000
Status:
Resolved
Fix version(s):
  • 1.9.3 (22/Sep/09)
Reporter:
Alex Zinchenko (admloki)
Assignee:
Luke Crouch (lcrouch)
Tags:
  • Zend_Rest_Route
Related issues:
Attachments:

Description

Good day, again.

I found a bug in Zend_Rest_Route match() method. The problem is in this code(Zend_Rest_Route::match(), line 150):


if ($pathElementCount && array_search($path[0], array('index', 'new')) > -1) {
    $specialGetTarget = array_shift($path);
} elseif ($pathElementCount == 1) {
    $params['id'] = array_shift($path);
} elseif ($pathElementCount == 0 || $pathElementCount > 1) {
    $specialGetTarget = 'index';
}

Because of lacks in $pathElementCount variable check 'match()' will only maps controller and action, so for path like '/page/edit/1' it will return this values.


//For params
array(1) { ["edit"]=>  string(1) "1" } 

//And for request something like that
array('controller' => 'page', 'action' => 'edit', 'module' => 'default', 'edit' => '1');

So, it will map id to 'edit' array key in iteration below, because action was not shifted from $path:


if ($numSegs = count($path)) {
    for ($i = 0; $i < $numSegs; $i = $i + 2) {
        $key = urldecode($path[$i]);
        $val = isset($path[$i + 1]) ? urldecode($path[$i + 1]) : null;
        $params[$key] = $val;
    }
}

Solution: patch Zend_Rest_Route on line 150:


if ($pathElementCount && array_search($path[0], array('index', 'new')) > -1) {
    $specialGetTarget = array_shift($path);
} elseif ($pathElementCount == 1) {
    $params['id'] = array_shift($path);
} elseif ($pathElementCount == 2 || $pathElementCount > 2) {
    if(is_numeric($path[0])) {
        //Somebody tries to access page by /controller/id/action
        $params['id'] = array_shift($path);
        $params['action'] = array_shift($path);
    } else {
        $params['action'] = array_shift($path);
        $params['id'] = array_shift($path);
    }
} elseif ($pathElementCount == 0 || $pathElementCount > 1) {
    $specialGetTarget = 'index';
}

Note: i marked it as blocker, because of impossibility in modifying 'match()' method from subclass - Zend_Rest_Route uses private methods, such as Zend_Rest_Route::_checkRestfulController().

Comments

Posted by Luke Crouch (lcrouch) on 2009-09-04T06:45:28.000+0000

This is expected behavior. 'edit' is a special target, as the code implies. To go to the editAction for a resource, you use the following URI:

/page/1/edit

Should route to PageController::editAction() with am 'id' param of 1.

Posted by Alex Zinchenko (admloki) on 2009-09-04T07:02:04.000+0000

No, it's not.

Key to value mapping appears before 'edit' target check:


// Digest URI params
if ($numSegs = count($path)) {
    for ($i = 0; $i < $numSegs; $i = $i + 2) {
        $key = urldecode($path[$i]);
        $val = isset($path[$i + 1]) ? urldecode($path[$i + 1]) : null;
        $params[$key] = $val;
    }
}

// Check for trailing "special get" URI
if (array_key_exists('edit', $params)) {
    $specialGetTarget = 'edit';
}

But 'edit' appears not as an array key, but value:


//match() result dump
array(3) { ["controller"]=>  string(4) "page" ["action"]=>  string(5) "index" [1]=>  string(4) "edit" }

But even change 'array_key_exists' to 'in_array' it willn't help for other actions.

Posted by Luke Crouch (lcrouch) on 2009-09-04T13:57:56.000+0000

If you want to invoke PageController::editAction($id = 1) you use this URI:

/page/1/edit

The paradigm for GET requests in the route is:

/{resource}/new or /{resource}/{identifier} or /{resource}/{identifier}/edit or /{resource}/index/{param_name}/{param_value}/{param_name}/{param_value}

To enable something like '/page/edit/1' as PageController::editAction is to break with the intended paradigm of the route.

Posted by Alex Zinchenko (admloki) on 2009-09-05T04:14:08.000+0000

Sometimes i get feeling nobody read what i wrote. Zend_Rest_Route will not map route like /page/1/edit.

It will map /page/edit/, but with wrong values:


array(3) { ["controller"]=>  string(4) "page" ["action"]=>  string(4) "edit" ["edit"]=>  string(1) "1" }

Posted by Luke Crouch (lcrouch) on 2009-09-06T18:57:41.000+0000

Ah, I see now. I think I lost what the issue was in all the code and commentary of the first comment. I have a test to reproduce, but I'm using a different fix that will preserve the index functionality, the textual identifiers, and not confuse the order of the action value and id values.

Posted by Luke Crouch (lcrouch) on 2009-09-06T19:03:58.000+0000

Should now be fixed so a URI like:

/page/1/edit

returns with from match with controller => page; id => 1; action => edit