# Getting 6 possible URL parameters

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:

• 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? ## 3 Answers Your regex could be improved: $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 loop

like:

if ($a == "in" &&$aa) { $searchLocation =$aa; }
else if ($a == "missinghtml") {$errorUrl = true; }

• That's much appreciated, thanks! I will be using commas in the URL, the use case being to search in multiple locations: - foo.com/do/design/in/london,liverpool/ Everything else I've updated and it's still working the same. Thank you. – Craig Jun 17 '15 at 13:44

In addition to the changes already proposed by Quill I strongly hope you reconsider the approach you're taking here.

I assign each section a variable ...

You can strongly simplify your code by just extracting a group match out of the URL:

$regex = '$/(in|at|do)/([^/]+)/$i';  Matching this regex will get you your $search... as the second group to extract.

under the assertion that a search-identifier will always be followed by it's value and not another identifier you can thus extract your values from the URL as follows:

$URL // here's your URL$locationRegex = '$/in/([^/]+)/$i';
$resultcontainer; if(preg_match($locationRegex, $URL,$resultcontainer)) {
$searchLocation =$resultcontainer[1];
}
// and similarly for the other variables


now we should put these two approaches together...

$url // here's your URL$pattern = '$/(in|at|do)/([^/]+)/$gi';
$resultContainer; if (preg_match($pattern, $url,$resultContainer)) {
// at this point your result container contains up to 6 elements:
for ($i = 0;$i < count($resulContainer) - 1;$i = $i + 2) { switch ($resultContainer[$i]) { case "in":$searchLocation = $resultContainer[$i + 1];
break;
case "at":
$searchCompany =$resultContainer[$i + 1]; break; case "do":$searchCategory = $resultContainer[$i + 1];
break;
}


This approach basically parses the URL tokens into "key-value-pairs" which might be better stored into a different array structure, but that depends on what you need to do with this later on ;)

• Shouldn't the regex be (in|at|do)/([^/]+)? – xyz Jun 17 '15 at 16:33
• @prakharsingh95 my PHP is not the best.. isn't $ a valid regex delimiter? – Vogel612 Jun 17 '15 at 16:35 • I meant the added +. Otherwise the second capture group would be l only for foo/in/london – xyz Jun 17 '15 at 16:36 • ohhh crap, yes you're completely right – Vogel612 Jun 17 '15 at 16:40 • Thanks guys, I've edited my original question with the updated code I'm using. – Craig Jun 17 '15 at 17:41 Thanks so much for your combined suggestions, my code looks like this: $regex = "/[^a-z0-9,-\/]/i";
$queryURL =$_SERVER['REQUEST_URI']."/";
$cleanQueryURL = preg_replace("".$regex."", "", $queryURL); if(preg_match('/in\/(.+?)\//i',$cleanQueryURL, $resultcontainer)) {$searchLocation = $resultcontainer[1]; } else {$searchLocation = "all"; }
if(preg_match('/at\/(.+?)\//i', $cleanQueryURL,$resultcontainer)) {
$searchCompany =$resultcontainer[1];
} else { $searchCompany = "all"; } if(preg_match('/do\/(.+?)\//i',$cleanQueryURL, $resultcontainer)) {$searchCategory = $resultcontainer[1]; } else {$searchCategory = "all"; }


I had to modify the regex slightly, but I used this - https://regex101.com/r/iZ8pV4/1 - and got there in the end!

You guys rock :)