statamic / v2-hub

Statamic 2 - Feature Requests and Bug Reports
https://statamic.com
95 stars 5 forks source link

Incorrect HTTP method causes error #2467

Closed simonhamp closed 4 years ago

simonhamp commented 4 years ago

Describe the bug When using add-on URLs that respond to a specific HTTP method (e.g. /!/MySexyAddon/SomeMethod => MySexyAddon\MySexyAddonController@postSomeMethod), any other HTTP method used to call that URL will cause an error.

To Reproduce Steps to reproduce the behavior:

  1. Create an addon
  2. Create a POST method in the addon
  3. Call the URL for that method using a GET request in your browser
  4. See error

Expected behavior I expect either a 404 or a 405 error.

Environment details (please complete the following information):

simonhamp commented 4 years ago

I've managed to patch this by changing the following methods in Statamic\Http\Controllers\StatamicController. Obviously would be better if this was in the core:


    /**
     * Call an addon's controller method and inject any dependencies
     *
     * @param string $name
     * @param string $method
     * @param array $parameters
     * @return mixed
     */
    private function callControllerMethod($name, $method, $parameters)
    {
        $studly = Str::studly($name);
        $method = strtolower($this->request->method()) . Str::studly($method);
        $namespace = "Statamic\\Addons\\$studly\\";
        $params = $parameters ?: [];

        // First check the root level controller, named after the addon.
        // eg. Statamic\Addons\AddonName\AddonNameController
        if (class_exists($rootClass = $namespace . "{$studly}Controller") && method_exists($rootClass, $method)) {
            return app()->call($rootClass.'@'.$method, $params);
        }

        // Next, check the controller namespace, still named after the addon.
        // eg. Statamic\Addons\AddonName\Controllers\AddonNameController
        if (class_exists($namespacedClass = $namespace."Controllers\\{$studly}Controller") && method_exists($namespacedClass, $method)) {
            return app()->call($namespacedClass.'@'.$method, $params);
        }
    }

    /**
     * Fire an event
     *
     * @param string  $namespace   URL segment 1
     * @param string  $event       URL segment 2
     * @param array   $parameters  Additional data
     * @return \Illuminate\Http\Response
     */
    private function fireEvent($namespace, $event, $parameters = [])
    {
        $response = array_get(Event::fire("{$namespace}.{$event}", $parameters), 0);

        // If a view has been returned from an event, we want to render it.
        if ($response instanceof \Illuminate\Contracts\View\View ||
            $response instanceof \Illuminate\Http\RedirectResponse ||
            $response instanceof \Illuminate\Http\Response
        ) {
            return $response;
        }
    }
simonhamp commented 4 years ago

@jasonvarga Thanks for applying a fix for this!

Just a note for others who may come across this later: the fix behaves as expected when APP_DEBUG=false