Skip to content

Instantly share code, notes, and snippets.

@dajve
Last active August 18, 2018 09:04
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save dajve/6c019809b30540a2b1fbffdd0e3c1bff to your computer and use it in GitHub Desktop.
Save dajve/6c019809b30540a2b1fbffdd0e3c1bff to your computer and use it in GitHub Desktop.
<?php
/*
* First we need to add checks for the existence of this param, as well
* as sanitizing the user submitted input
* The try catch is so this scratch continues to run with an example value
*/
try {
$url = filter_input(INPUT_GET, 'url', FILTER_SANITIZE_STRING);
if (empty($url)) {
throw new RuntimeException("No URL param");
}
} catch (Exception $e) {
$url = "Pabel/name/";
}
/*
* Original:
$url = rtrim($url, '/');
$url = explode('/', $url);
*/
/*
* Let's use a different variable name so we don't get confused when
* switching from a string to an array
* Also, let's ensure we trim out the whitespace from the resulting parts
*/
$urlParts = array_map('trim', explode('/', trim($url, ' /')));
// Now, let's protect against future directory traversal by replacing filepath parts
$urlParts = array_map(function($urlPart) {
return str_replace(['.', '/', '\\'], null, $urlPart);
}, $urlParts);
/*
* Add some more checking that we receive the correct number of parts
*/
switch (count($urlParts)) {
case 2 :
// This is what we want. No action here
break;
case 1 :
// Maybe you could handle a default route here?
$urlParts[] = 'index';
break;
case 0 :
throw new RuntimeException("No valid URL parts found");
break;
default :
// It's up to you whether you're good with being sent too many parts
// I'm trashing here but you can default this line and merge with
// case 2 if this is OK
throw new RuntimeException("Encountered too many URL parts");
break;
}
/*
* Original:
include 'app/controllers/'.$url[0].'.php'
*/
/*
* First of all, don't use include here. Include will not stop execution
* if the file isn't found. It also won't care if you've already included
* the file, leading to multiple definitions of the containing class - bad times
*/
$includePath = sprintf(
'app%scontrollers%s%s.php',
DIRECTORY_SEPARATOR, // In case the directory separator isn't /
DIRECTORY_SEPARATOR,
$urlParts[0]
);
// And make sure exists (yes, require will handle that for us, but this way
// we are in control of what happens in this scenario rather than just dying
// It also helps check for NULL bytes; see: https://bugs.php.net/bug.php?id=39863
if (!file_exists($includePath)) {
throw new RuntimeException(sprintf("Cannot find controller for %s", $urlParts[0]));
}
require_once($includePath);
/*
* Original:
$controller = new $url[0]();
*/
$controllerName = $urlParts[0];
// Ensure that the controller exists in the included file
if (!class_exists($controllerName)) {
throw new RuntimeException(sprintf("Could not find controller '%s'", $controllerName));
}
$controller = new $controllerName();
/*
* Original:
$controller->$url[1]();
*/
/*
* This is where the issue in the OP occurred.
* It happens because PHP reads the statement from left to right and you aren't
* grouping with curly braces
* What your original translates to at the moment is
* $controller->Array[1]()
* because the $url variable is expanded first
*
* You could use
$controller->{$url[1]}();
* to inform it that the $url[1], in its entirety, is the action name, but
* the following is more verbose and unambiguous
*/
$actionName = $urlParts[1];
// Ensure that the action exists
if (!method_exists($controller, $actionName)) {
throw new RuntimeException(sprintf("Could not find action '%s' in controller '%s'", $actionName, $controllerName));
}
$controller->{$actionName}();
/// EDIT
// An additional note would be to use a naming convention for action methods, so you don't
// allow people to fire methods which aren't routes
// For example, if Pabel::deleteEverything() had been defined, you wouldn't want people
// being able to trigger it by passing index.php?url=Pabel/deleteEverything/
// Consider making all action method names end with "Action" (eg Pabel::nameAction())
// and making line 109 above as follows:
// $actionName = sprintf("%sAction", $urlParts[1]);
// In our delete example, that would then fail then check in L:111
<?php
/**
* File: app/controllers/Pabel.php
* Class Pabel
*/
class Pabel
{
/**
* @return void
*/
public function name()
{
echo "Hello, I am Pabel Ahmed";
}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment