do I put the exit(); after each PHP header location or just after the last if statement?

Question

Just as the title says, Wondering if I need to put the exit(); after each PHP if statement, or just the main if statement or just the last if statement? Currently I have it on the last if statement and it works fine.

if ( isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) {

    if ( strpos($_SERVER['QUERY_STRING'], 'occupation') === false || $_SERVER['REQUEST_URI'] != strtolower($_SERVER['REQUEST_URI']) || $occupation == '' ) {
        header ('Location: http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position), true, 301);
    }

    if ( $arr_position == '' ) {
        header ('Location: http://' . $domain . '/jobs/', true, 301);
    }

    if ( isset($home) && $arr_position == '' ) {
        header ('Location: http://' . $domain, true, 301);
    }

    if ( isset($home) && $arr_position != '' ) {
        header ('Location: http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position), true, 301);

        exit();
    }
}

UPDATE

I changed it to have an exit() after each header location call and all is still working fine. This is in my main template file, so yes there is code below that needs execution if nothing matches in the header location if block.

I now have it like this

if ( isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) {

    if ( strpos($_SERVER['QUERY_STRING'], 'occupation') === false || $_SERVER['REQUEST_URI'] != strtolower($_SERVER['REQUEST_URI']) || $occupation != strtolower($arr_occupation) ) {
        header ('Location: http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position), true, 301);
        exit();
    }

    if ( $arr_position == '' ) {
        header ('Location: http://' . $domain . '/jobs/', true, 301);
        exit();
    }

    if ( isset($home) && $arr_position == '' ) {
        header ('Location: http://' . $domain, true, 301);
        exit();
    }

    if ( isset($home) && $position == urlencode($arr_position) ) {
        header ('Location: http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position), true, 301);
        exit();
    }
}

UPDATE 2

I went ahead and used DefiniteIntegral answer because I like the logic, but I had to use separate if statements within the main block. Using elseif does not work. I have fully tested it and using different conditions in each elseif and it just won't work right. So I am fine with using separate if statements within the main block.

Here is how it looks now

if ( isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) {
    $new_url = false;

    if ( strpos($_SERVER['QUERY_STRING'], 'occupation') === false || $_SERVER['REQUEST_URI'] != strtolower($_SERVER['REQUEST_URI']) || $occupation != strtolower($arr_occupation) ) {
        $new_url = 'http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position);
    } 

    if ( $arr_position == '' ) {
        $new_url = 'http://' . $domain . '/jobs/';
    }

    if ( isset($home) && $arr_position == '' ) {
        $new_url = 'http://' . $domain;
    }

    if ( isset($home) && $arr_position != '' ) {
        $new_url = 'http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position);
    }

    if ($new_url !== false) {
        header('Location: ' . $new_url, true, 301);
        exit();
    }
}

FINAL UPDATE

Finally got it working right with the elseif statements. Had to add more conditions to the if statements in each elseif block of code.

Final code looks like this

if ( isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) {
    $new_url = false;

    if ( !isset($occupation) || $occupation != strtolower($arr_occupation) || $_SERVER['REQUEST_URI'] != strtolower($_SERVER['REQUEST_URI']) ) {
        $new_url = 'http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position);

    } elseif ( isset($home) ) {
        if ( !isset($position) || $position != $arr_position || $position == '' ) {
            $new_url = 'http://' . $domain;
        } else {
            $new_url = 'http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position);
        }

    } elseif ( $currentPage == 'Jobs' ) {
        if ( !isset($position) || $position == '' ) {
            $new_url = 'http://' . $domain . '/jobs/';
        } elseif (isset($position) && $position != '' && $position != $arr_position) {
            $new_url = 'http://' . $domain . strtolower ( '/jobs/?occupation='.$arr_occupation.'&position='.$arr_position);
        }
    }

    if ($new_url !== false) {
        header('Location: ' . $new_url, true, 301);
        exit();
    }
}

DefiniteIntegral · Answer 1 · 2014-10-01T19:38:30.300

2

If your goal is to always perform a redirect in this block, put your exit statement after the last if block, outside the braces. If there is code below the block you showed us, it will still execute unnecessarily if the goal is a redirect.

if ( isset($_SERVER['QUERY_STRING']) && !empty($_SERVER['QUERY_STRING']) ) {
    $new_url = false;

    if ( strpos($_SERVER['QUERY_STRING'], 'occupation') === false || $_SERVER['REQUEST_URI']    != strtolower($_SERVER['REQUEST_URI']) || $occupation == '' ) {
        $new_url = 'http://' . $domain . strtolower ( '/ jobs/?occupation='.$arr_occupation.'&position='.$arr_position);
    } elseif ( isset($home) ) {
        if ( $arr_position == '' ) {
            $new_url = 'http://' . $domain;
        } else {
            $new_url = 'http://' . $domain . strtolower ( '/ jobs/?occupation='.$arr_occupation.'&position='.$arr_position);    
        }
    } elseif ( $arr_position == '' ) {
        $new_url = 'http://' . $domain . '/jobs/';
    }

    if ($new_url !== false) {
        header('Location: ' . $new_url, true, 301);
        exit;
    }
}

edited Oct 01 '14 at 19:38

answered Oct 01 '14 at 02:24

DefiniteIntegral

579
3
6

I tried it like that already but site doesn't work when there is a query string in the url as that is the first if condition in the main block. – Mike Oct 01 '14 at 02:35
Sorry, not totally sure what you're trying to accomplish. See edit above if you're only trying to perform a redirect if a condition matches. – DefiniteIntegral Oct 01 '14 at 02:45
ok I like how you structured this block. But when I write it with `elseif` instead of each separate `if` in the main block, then one of the elseif does not get fired. This one never fires, always the one right above it does even when home is set. `elseif ( isset($home) && $arr_position == '' )` – Mike Oct 01 '14 at 03:24
could you tell me why that is, regarding my previous comment above? – Mike Oct 01 '14 at 19:32
Okay, actually reading the conditions through, it makes sense you'd need to change the structure to do the same as the previous code. if...elseif... will evaluate the first condition that passes, so it is a better design if you can get the logic right. Updated above again. – DefiniteIntegral Oct 01 '14 at 19:36
Another thing I'd just like to call out to you—in most environments you should already have access to $_GET, a superglobal array that stores key-value pairs for query string variables, already decoded for you. – DefiniteIntegral Oct 01 '14 at 19:39
So for example, strpos($_SERVER['QUERY_STRING'], 'occupation') === false vs. isset($_GET['occupation']) should be the same. – DefiniteIntegral Oct 01 '14 at 19:40
tried your updated block, still the same. I even put a new condition on the last `elseif` to say `} elseif ( $currentPage == 'Jobs' ) { if ( $arr_position == '' ) { $new_url = 'http://' . $domain . '/jobs/'; } }` and still get same output. This bit never fires `} elseif ( isset($home) ) { if ( $arr_position == '' ) { $new_url = 'http://' . $domain;` – Mike Oct 01 '14 at 21:03
got it working right finally. I added a final update to my question above if you want to see what I had to do. – Mike Oct 02 '14 at 01:48

score 1 · Answer 2 · answered Oct 01 '14 at 02:36

In your case after each header() call: You have 4 different conditions and none or more might match so you could be executing multiple header() calls or none at all when you have passed your last if statement, causing your page to exit without any redirect at all.

If it was a block of one if statement followed with 1 or more elseif statements that ended with an else, all containing redirects, you could put it at the end:

if
  header();
elseif
  header();
elseif
  header();
else
  header();
exit;

But that would be the only scenario as you would need some sort of error handling if none of the if statements in your current code is not matched.

do I put the exit(); after each PHP header location or just after the last if statement?

2 Answers2