@Zeichen32 opened this Issue on July 24th 2015 Contributor

I've noticed that Events triggered by the FrontController Dispatcher are case-sensitive, but the called actions are is case-insensitive.

For example:
?module=CoreAdminHome&action=optout => triggers action CoreAdminHome:_optOut_
?module=CoreAdminHome&action=optOut => triggers action CoreAdminHome:_optOut_
?module=CoreAdminHome&action=OptOut => triggers action CoreAdminHome:_optOut_

Events:
?module=CoreAdminHome&action=optout => triggers event Controller.CoreAdminHome._optout_
?module=CoreAdminHome&action=optOut => triggers event Controller.CoreAdminHome._optOut_
?module=CoreAdminHome&action=OptOut => triggers event Controller.CoreAdminHome._OptOut_

So the docs or the behavior is wrong:

* Triggered directly before controller actions are dispatched.
*
* This event exists for convenience and is triggered directly after the {<a class='mention' href='https://github.com/hook'>@hook</a> Request.dispatch}
* event is triggered.
*
* It can be used to do the same things as the {<a class='mention' href='https://github.com/hook'>@hook</a> Request.dispatch} event, but for one controller
* action only. Using this event will result in a little less code than {<a class='mention' href='https://github.com/hook'>@hook</a> Request.dispatch}

File: https://github.com/piwik/piwik/blob/master/core/FrontController.php#L492

@tsteur commented on July 24th 2015 Owner

I'm a bit surprised to are all rendered perfectly. We should always use the correct action name as defined in the code or alternatively always lowercase I don't really mind :) We should have a look why it renders all of those actions when only one should be valid

@Zeichen32 commented on July 24th 2015 Contributor

Its because php functions are case-insensitive.

Note: Function names are case-insensitive, though it is usually good form to call functions as they appear in their declaration.

http://php.net/manual/en/functions.user-defined.php

The easiest way were to convert the controller action name to lowercase. But it's maybe a BC.

@mattab commented on July 24th 2015 Owner

I would suggest that &action= should match the controller action as found in code (case sensitive) VS using lowercase only

Edit: this may also be BC breaking for some users, but I suppose only the few who would have used the wrong case for &action in their Widgetized URL or wrong case in events listeners

@Zeichen32 commented on July 24th 2015 Contributor

If we change the ControllerResolver it can be done case-sensitive:

https://github.com/piwik/piwik/blob/master/core/Http/ControllerResolver.php#L74

/** <a class='mention' href='https://github.com/var'>@var</a> $controller Controller */
$controller = $this->abstractFactory->make($controllerClass);

$action = $action ?: $controller->getDefaultAction();

if (!is_callable(array($controller, $action)) || !in_array($action, get_class_methods($controller))) {
    return null;
}
@tsteur commented on July 24th 2015 Owner

:+1: sounds good, should also not cause any performance issues

@Zeichen32 commented on July 24th 2015 Contributor

Ive create a PR to fix this issue #8426

@tsteur commented on July 24th 2015 Owner

We could afterwards write a test that gets a list of all events (starting with Controller.) of all plugins and check whether the controller actions actually exist. This way we make sure we don't break anything and will get notified easily in case one has a typo in the future

@Zeichen32 commented on July 24th 2015 Contributor

I will add some tests

@tsteur commented on April 4th 2016 Owner

This issue was closed via #8426

@tsteur commented on August 8th 2016 Owner

From my understanding this issue was fixed

This Issue was closed on August 8th 2016
Powered by GitHub Issue Mirror