parameter passing issue on function

[Azarian]

Well I thought I had created a pretty slick web page, using ?page and $_GET to load pages which where really function calls.

 

 

Here is a few lines that make up the menu

<li><a Href="?page=welcome_page">Home</a></li>
<li><a Href="?page=laninfo_display">Party Info</a></li>

 

This function loads other functions into the main body of the web page.

function main_page(){
if ( $_GET['page'] == null ) {
	$menu_link = welcome_page;
	$menu_link();
}else{		
	$menu_link = $_GET['page'];
	$menu_link();
}		
}

 

All was good till recently I needed to start passing parameters back and forth between some functions.  What would be the best way to solve this issue?

[trq]

Well I thought I had created a pretty slick web page, using ?page and $_GET to load pages which where really function calls.

 

I would be very careful doing that, especially the way your code works. Theres nothing stopping anyone executing whatever function they like.

[Azarian]

Well I thought I had created a pretty slick web page, using ?page and $_GET to load pages which where really function calls.

 

I would be very careful doing that, especially the way your code works. Theres nothing stopping anyone executing whatever function they like.

 

How would they do that and how would I stop them?

[trq]

They would do that by passing ?page=mysql_query (using mysql_query as an example) to your script.

 

To fix it, you should store a list of valid (aloud) functions within an array, then check that prior to executing the function.

[Azarian]

They would do that by passing ?page=mysql_query (using mysql_query as an example) to your script.

 

To fix it, you should store a list of valid (aloud) functions within an array, then check that prior to executing the function.

 

Your assuming that they would be able to guess the names of my functions.  Plus if by some miracle they guessed some function related to admin panel they wouldn't have the authorization needed to access it.  Seems counter intuitive of what I am trying to do buy having an array that has hundreds of other arrays listed in it, also this doesn't fix the issue I am having. 

[trq]

Your assuming that they would be able to guess the names of my functions.

 

They wouldn't be trying to execute your functions. There's nothing stopping them executing built in functions either.

 

And, the way your heading (allowing args to be parsed to these functions) will open even more holes. There would be nothing stoping someone who knows what they are doing deleting your site / changing passwords to your databases / gaining access to your database, pretty much whatever they like.

 

Don't believe me? That's your problem.

 

As for your issue at hand, I'm not exactly sure what the actual problem is.

[Azarian]

the way your heading (allowing args to be parsed to these functions)

 

That is the problem it doesn't allow args to be parsed.

[trq]

Your functions have access to the $_GET array. They can easily get any arguments from there if required.

[Azarian]

Your functions have access to the $_GET array. They can easily get any arguments from there if required.

 

lets say I need to call Somefunction($somevariable); How would I do it?

[trq]

Your functions have access to the $_GET array. They can easily get any arguments from there if required.

 

lets say I need to call Somefunction($somevariable); How would I do it?

 

With your current design you wouldn't. Your functions themselves are going to need to look through the $_GET array for any params that might effect them.

[trq]

A simple example (using something like your design).

 

Lets say we have two functions.

 

function foo()
{
}

function bar()
{
}

 

Now, lets say foo() might required an id to be passed to it, it has access to the get array so....

 

function foo()
{
  if (isset($_GET['id'])) {
    $id = $_GET['id'];
    // now use $id
  }
}

 

So now if you call ?page=foo&id=2 foo() will get access to 2.

 

You could also place your logic within the minimal front controller you've already implemented but in reality, it makes more sense for the functions themselves to look for what they need.

[Azarian]

Oh ok I see where I am going wrong now.

 

Ok now that we got my problem solved lets chat about this $_GET array.  Thinking out side of the box here.  How could I rewrite this better and not have some big crazy array listed somewhere to check against?  I don't suppose there is a function that would go through see all the other functions dynamically?

[trq]

All you would need to do is put a list of valid functions in an array, then check it.

 

function main_page(){
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
      if (in_array($_GET['page'], $valid)) {
        $menu_link = $_GET['page'];
        $menu_link();
      }
   }      
}

 

Another option, would be to prefix all your valid functions with a prefix. eg; instead of foo() use actionFoo().

 

function main_page() {
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
        $menu_link = 'action' . ucfirst($_GET['page']);
        $menu_link();
   }      
}

 

You really should be checking if these functions are callable first too...

 

function main_page() {
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
      $menu_link = 'action' . ucfirst($_GET['page']);
      if (is_callable($menu_link)) {
        $menu_link();
      }
   }      
}

 

There really is plenty to do with your design, but overall its a decent setup. You just need to refine a few things.

[Azarian]

Ok I like the action example better!  When the is_callable function is called what is it testing the variable against to know if it is valid funtion or not?

[trq]

is_callable.

[Azarian]

is_callable.

 

Yea I already looked it up on that page and it doesn't answer my question.

[Azarian]

If I prefix all my functions with action_ for example.

 

$menu_link = 'action_' . ucfirst($_GET['page']);

 

This line of code wouldn't make any sense.  If my function name is action_somefunction this code would really make it action_Action_somefunction. 

 

Although if I understand correctly what we are trying to accomplish.  I shouldn't prefix any of my functions with anything and use this code.

 

$menu_link =  'action_' . $_GET['page'];
if (is_callable($menu_link)) {
     $menu_link();
}

 

I don't need the function to be uppercased.  This is where the whole is_callable would finally make some sense now.  If someone tried to call a built in function it would fail because action_builtinfunction would never exist.  Right?

 

[Azarian]

Well I am only particailly right it seems.  Removing all the prefixes isn't the answer either.  This code doesn't see my functions than.

 

$menu_link =  'action_' . $_GET['page'];
if (is_callable($menu_link)) {
     $menu_link();
}

 

To make it all work properly this is the code that would have to be used whether or not I prefix all the functions.  Which brings us back to where I already was basically.

 

$menu_link =  $_GET['page'];
if (is_callable($menu_link)) {
     $menu_link();
}

[trq]

With this code.

 

]
function main_page() {
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
      $menu_link = 'action' . ucfirst($_GET['page']);
      if (is_callable($menu_link)) {
        $menu_link();
      }
   }      
}

 

Your functions would all need to be....

 

actionFoo() {}
actionBar() {}

 

etc etc. PHP isn't case sensitive when it comes to functions though so they could just as easily be actionsfoo(), its just more of a standard to use camel case for function names.

 

Then, your url's would be unchainged from your previous code. eg; ?page=foo would execute actionFoo().

 

The safe quard here is they someone passing ?page=mysql_query would end up trying to execute actionMysql_query() which doesn't exist and would therefore not pose a security risk.

[salathe]

This is slightly tangential to the current discussion but addressing:

I don't suppose there is a function that would go through see all the other functions dynamically?

 

Sure, get_defined_functions (in particular the "user" array returned from that; unless I missed your point entirely  ).

[Azarian]

This is slightly tangential to the current discussion but addressing:

I don't suppose there is a function that would go through see all the other functions dynamically?

 

Sure, get_defined_functions (in particular the "user" array returned from that; unless I missed your point entirely  ).

 

Yea actually according to the description that is totally what I was asking for but looking at the examples i am kinda lost how to use it.  LOL

[trq]

It returns an array. What exactly do you want to use it for? To check that your function is trying to execute a user defined function and not a built in?

 

That would be....

 

function main_page(){
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
      $defined = get_defined_functions();
      $valid = $defined['user'];
      if (in_array($_GET['page'], $valid)) {
        $menu_link = $_GET['page'];
        $menu_link();
      }
   }      
}

 

I'm thinking however thay the prefix option is probably your best bet. See my previous post.

[Azarian]

 

function main_page() {
   if (!isset($_GET['page']))) {
      $menu_link = "welcome_page";
      $menu_link();
   } else {
      $menu_link = 'action' . ucfirst($_GET['page']);
      if (is_callable($menu_link)) {
        $menu_link();
      }
   }      
}

 

I understand what you are trying to do but this code won't work at all.  I think I will have to think about it awhile and rewrite this function differently.  If I use !isset instead of checking if $_GET is null or not it will never load the welcome_page function.  This code 'action' . ucfirst($_GET['page']) is nice for stopping built it functions from being executed but it also stops my own function from being executed as well. 

 

Actually if we did a check first to see the function name was already prefixed with action this would be a better way wouldn't it?

[trq]

If I use !isset instead of checking if $_GET is null or not it will never load the welcome_page function.

 

You welcome page would be called whenever someone accesses your site without ?page being tacked on the end. You could over course re-write this to handle an empty ?page=

 

if (!isset($_GET['page']) || empty($_GET['page'])) {

 

This code 'action' . ucfirst($_GET['page']) is nice for stopping built it functions from being executed but it also stops my own function from being executed as well.

 

Not if (like I said) all your functions are prefixed with action* as in....

 

actionFoo() {}
actionBar() {}

 

This is how most major frameworks handle the exact same issue, though they also have these functions contained within controller classes.

[Azarian]

If I use !isset instead of checking if $_GET is null or not it will never load the welcome_page function.

 

You welcome page would be called whenever someone accesses your site without ?page being tacked on the end. You could over course re-write this to handle an empty ?page=

 

if (!isset($_GET['page']) || empty($_GET['page'])) {

 

This code 'action' . ucfirst($_GET['page']) is nice for stopping built it functions from being executed but it also stops my own function from being executed as well.

 

Not if (like I said) all your functions are prefixed with action* as in....

 

actionFoo() {}
actionBar() {}

 

This is how most major frameworks handle the exact same issue, though they also have these functions contained within controller classes.

 

When I renamed all the functions prefixed with action.

 

function action_somefunction(){}

 

When that code would execute it would return this.

 

action_Action_somefunction

 

This isn't what we want.