Skip to content
This repository has been archived by the owner on Feb 13, 2022. It is now read-only.

Callable stops execution of router #116

Closed
Sinevia opened this issue Oct 21, 2018 · 9 comments
Closed

Callable stops execution of router #116

Sinevia opened this issue Oct 21, 2018 · 9 comments
Assignees
Labels

Comments

@Sinevia
Copy link

Sinevia commented Oct 21, 2018

Calling a non-static method using the default callable implementation always causes the constructors to be called, which is undesirable. See in the example below even if you call the / route, the execution will be stopped as the constructor in test will be called.

class TestController {
    function __construct() {
         if (has_access() == false){
             die('No Access');
         }
    }
    function page() {
        return "Page A";
    }
}
Route\get('/user', [(new TestController), "page"]);
Route\get('/', function(){
    return "Home";   
});

I would suggest a workaround for calling non-static methods

Route\get('/user', "TestController@page");

@leocavalcante leocavalcante self-assigned this Oct 21, 2018
@leocavalcante
Copy link
Owner

Well, actually this isn't a bug. The constructor of TestController will always be called, indeed, you're doing it at [(new TestController), "page"], it is not something on the Siler internals... We can't prevent or predict user code inside a lib/framework.

@Sinevia
Copy link
Author

Sinevia commented Oct 21, 2018

I agree it cannot be prevented, this is why I propose a workaround that will work with non-static methods without side effects

@leocavalcante
Copy link
Owner

Handling class construction isn't a Siler concern, at least for now, you should look for something like Pimple. Yet you could just wrap it in a Closure:

Route\get('user', function () {
  $c = new TestController();
  return $c->page();
});

@Sinevia
Copy link
Author

Sinevia commented Oct 22, 2018

@leocavalcante this is actually a problem as the documentation says the router can use any callable. In this case it is a problem as it causes code execution that the user will be unaware of. I agree its not a problem with Siler per se, but this is the way this particular callable works. If Siler decides not to address this, this is fine, but the documentation needs to be updated to caveat this case.

I believe other routers have recognized this as a problem and have come up with a solution:

image

Example at:
https://github.com/magnus-eriksson/router

3 similar comments
@Sinevia
Copy link
Author

Sinevia commented Oct 22, 2018

@leocavalcante this is actually a problem as the documentation says the router can use any callable. In this case it is a problem as it causes code execution that the user will be unaware of. I agree its not a problem with Siler per se, but this is the way this particular callable works. If Siler decides not to address this, this is fine, but the documentation needs to be updated to caveat this case.

I believe other routers have recognized this as a problem and have come up with a solution:

image

Example at:
https://github.com/magnus-eriksson/router

@Sinevia
Copy link
Author

Sinevia commented Oct 22, 2018

@leocavalcante this is actually a problem as the documentation says the router can use any callable. In this case it is a problem as it causes code execution that the user will be unaware of. I agree its not a problem with Siler per se, but this is the way this particular callable works. If Siler decides not to address this, this is fine, but the documentation needs to be updated to caveat this case.

I believe other routers have recognized this as a problem and have come up with a solution:

image

Example at:
https://github.com/magnus-eriksson/router

@Sinevia
Copy link
Author

Sinevia commented Oct 22, 2018

@leocavalcante this is actually a problem as the documentation says the router can use any callable. In this case it is a problem as it causes code execution that the user will be unaware of. I agree its not a problem with Siler per se, but this is the way this particular callable works. If Siler decides not to address this, this is fine, but the documentation needs to be updated to caveat this case.

I believe other routers have recognized this as a problem and have come up with a solution:

image

Example at:
https://github.com/magnus-eriksson/router

@Sinevia
Copy link
Author

Sinevia commented Oct 22, 2018

Sorry for reposting so many times. Github was showing 500 page and unable to post message.

@leocavalcante
Copy link
Owner

NP, I got some buggy behavior from Github too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants