patternphpMinor
Getting 6 possible URL parameters
Viewed 0 times
possibleparametersurlgetting
Problem
I am in the midst of writing a simple job board and I have the URL's filtering and working how I want, everything works - I just want to know if I can improve it, if there are any security concerns, and if it is horribly inefficient.
Take the following URL section - foo.com/in/london/at/google/do/design
I assign each section a variable and work out the location, company, and category.
Other use cases that work:
My code to figure out all these variables is:
I'm looking at this thinking there must be a better to do this. Is this section secure? Any thoughts on this are much appreciated. Like I say it works, but can it be better?
Take the following URL section - foo.com/in/london/at/google/do/design
I assign each section a variable and work out the location, company, and category.
Other use cases that work:
- Switching the order - foo.com/at/google/in/london/do/design
- Having less parameters - foo.com/in/london/at/google
My code to figure out all these variables is:
$regex = "[^a-zA-Z0-9,-]";
$a = isset($_GET["a"]) ? preg_replace("/".$regex."/", "", $_GET["a"]) : "";
$aa = isset($_GET["aa"]) ? preg_replace("/".$regex."/", "", $_GET["aa"]) : "";
$b = isset($_GET["b"]) ? preg_replace("/".$regex."/", "", $_GET["b"]) : "";
$bb = isset($_GET["bb"]) ? preg_replace("/".$regex."/", "", $_GET["bb"]) : "";
$c = isset($_GET["c"]) ? preg_replace("/".$regex."/", "", $_GET["c"]) : "";
$cc = isset($_GET["cc"]) ? preg_replace("/".$regex."/", "", $_GET["cc"]) : "";
if ($a == "in" && $aa != null) { $searchLocation = $aa; }
if ($b == "in" && $bb != null) { $searchLocation = $bb; }
if ($c == "in" && $cc != null) { $searchLocation = $cc; }
if ($a == "at" && $aa != null) { $searchCompany = $aa; }
if ($b == "at" && $bb != null) { $searchCompany = $bb; }
if ($c == "at" && $cc != null) { $searchCompany = $cc; }
if ($a == "do" && $aa != null) { $searchCategory = $aa; }
if ($b == "do" && $bb != null) { $searchCategory = $bb; }
if ($c == "do" && $cc != null) { $searchCategory = $cc; }
if ($a == "missinghtml") { $errorUrl = true; }I'm looking at this thinking there must be a better to do this. Is this section secure? Any thoughts on this are much appreciated. Like I say it works, but can it be better?
Solution
Your regex could be improved:
The character
You don't really need to add
I see a lot of people do
Becomes:
Using
With
like:
$regex = "[^a-zA-Z0-9,-]";
$a = isset($_GET["a"]) ? preg_replace("/".$regex."/", "", $_GET["a"]) : "";The character
, shouldn't be in a URL, but _ can be.You don't really need to add
/ to the beginning and end of your regex every time.I see a lot of people do
a-zA-Z when they could really just do a-z with a case insensitive search. Becomes:
$regex = "/[^a-z0-9-_]/i"
^ ^^
$a = isset($_GET["a"]) ? preg_replace($regex, "", $_GET["a"]) : "";Using
if ($a == "in" && $aa != null) could probably be improved also as if $aa is not null, then it evaluates to true anyway:if ($a == "in" && $aa)With
if ($a == "missinghtml") { $errorUrl = true; }, you should make it into an else if looplike:
if ($a == "in" && $aa) { $searchLocation = $aa; }
else if ($a == "missinghtml") { $errorUrl = true; }Code Snippets
$regex = "[^a-zA-Z0-9,-]";
$a = isset($_GET["a"]) ? preg_replace("/".$regex."/", "", $_GET["a"]) : "";$regex = "/[^a-z0-9-_]/i"
^ ^^
$a = isset($_GET["a"]) ? preg_replace($regex, "", $_GET["a"]) : "";if ($a == "in" && $aa)if ($a == "in" && $aa) { $searchLocation = $aa; }
else if ($a == "missinghtml") { $errorUrl = true; }Context
StackExchange Code Review Q#93862, answer score: 4
Revisions (0)
No revisions yet.