PHP - MVC - Better way to prevent calling unwanted method in Controller?

Question

I don't know if my question should be asked here or not. Please let me know or move it/delete it if that is the case.

Question:

For the sake of learning, I'm making my own little MVC "library" for a very small personal website. Here is the process being used (please correct me if I'm not even close to doing it the right way):

All requests (except images, etc.) get sent through index.php (boostrap file).
The bootstrap file parses the request and extracts the controller and action, ex:

http://www.Domain.com/Controller/Action/Argument1/Argument2/...

Where Controller is the controller, action is the method called on the controller. In this case, it would end up being: Controller->Action ( Argument1, Argument2 );

Now, what if a user visits:

http://www.Domain.com/Controller/__destruct

or

http://www.Domain.com/Controller/__get/password

Current solution(s):

Run the request through a $config->getURIFilter () method or something
Do a check for method_exists () and is_callable (), etc.
Don't have any methods in the controller that aren't for handling a request

It just seems like this shouldn't be an issue to begin with and my design is wrong.

P.S. I've already looked at plenty of good MVC PHP frameworks (CodeIgniter, Yii, CakePHP, Zend, Swiftlet, etc.)

You might find [this question](http://stackoverflow.com/questions/3430181/acl-implementation) relevant. it is about access control in MVC. — tereško, Apr 15 '12 at 22:36

Gordon · Accepted Answer · 2012-04-15T07:53:02.697

2

Either make your Controllers only handle specific actions, e.g.

/controllers
 /user
  loginController.php
  logoutController.php

and then have classes that only do that one thing

class LoginController implements RequestHandler
{
    public function handleRequest(Request $request, Response $response)
    {
        …
    }

    private function someAuxiliaryMethod() {
        …

so that example.com/user/login will create a new LoginController and call the interface method handleRequest. This is a Strategy Pattern.

Or - if you dont want to split your controller actions like this - suffix your actions with a word (Zend Framework does this):

class UserController
{
    public function loginAction() {
        …

and in your bootstrap you add the suffix and invoke those methods then.

Yet another option would be to introduce a Router that can map URLs to Controller Actions and which can optionally do sanitizing to filter out malformed URLs, e.g. strip underscores.

edited Apr 15 '12 at 07:53

answered Apr 15 '12 at 07:37

Gordon

312,688
75
539
559

Thanks, Gordon. I love those elegant solutions. I've lurked here a lot and I'm glad I got an answer from you. I'm usually following your advice since I come here to search a lot for questions on good design, etc. and I prefer your methods personally. I think I might go with your first solution after reading more on the Stratgey pattern and do some reading on it as well. Thanks again. Edit: on 2nd thought, I like the 2nd one better for its simplicity and not having to make each controller a request handler (even though that's what a controller is anyway), seems like less work in each controller – Daniel Elkins Apr 15 '12 at 12:39
@DanielElkins you are welcome. And happy to learn my ramblings on SO are useful to people. Thanks :) – Gordon Apr 15 '12 at 12:43

score 0 · Answer 2 · answered Apr 15 '12 at 07:31

0

I suggest that you treat any method preceded with an underscore as a private method (just show a not found page when you do your method_exists check if it starts with an underscore).

answered Apr 15 '12 at 07:31

Adam

2,948
10
43
74

score 0 · Answer 3 · answered Apr 15 '12 at 07:38

0

Just mark methods that you don't want to expose to your controller as private or protected.

Keep it simple!

answered Apr 15 '12 at 07:38

Christophe Eblé

8,071
3
33
32

in general yes, but wont work in the case of `__get` like the OP shows – Gordon Apr 15 '12 at 07:47
`if(substr($method, 0, 2) == '__') { return; }` – ThiefMaster Apr 15 '12 at 08:01

score 0 · Answer 4 · answered Apr 15 '12 at 08:00

I rolled my own MVC structure once for the same reason you're doing it, and the way I solved this was simply making an architectural decision to prefix all routable controller actions with the word "action", ie:

public function actionGetData(){

}

public function actionUpdateSomething() {

}

etc.

The reasons I did this:

You maintain full control over public/protected/private scope in your class without worrying about whether a method is inadvertently exposed via some URL.
It makes the routable actions obvious to the programmer. public function actionDoSomething() is clearly accessible via a public URL (/controllerName/doSomething), whereas public function getErDone() is not.
You're not forced to jump through hoops to identify routable actions. Simply prefix your incoming parameter with "action" and see if the method exists. Fast and simple.

I found this worked really well.

PHP - MVC - Better way to prevent calling unwanted method in Controller?

4 Answers4