What's wrong with this simple PHP code from a form?

[fuzzybabybunny]

I have a user fill in a form.

There is a "gallery address" field and a "zip" field.

If the gallery address field is filled in (http://www.gallery.com), I want the output of the form to be a link to http://www.gallery.com

If the gallery address field is not filled, and the zip field is filled (123whatever.zip), I want the output of the form to be a link to the zip file - http://www.victorlinphoto.com/photodownloads/123whatever.zip


$photos="none"
if ($_POST["gallery"]=="")
{
$photos="http://www.victorlinphoto.com/photodownloads/" . $_POST["zip"];
}
else
{
$photos=$_POST["gallery"];
}

 

[Aluvus]

It would be useful to know what it is actually doing. I'm no PHP ninja, but I suspect the if statement is not testing true when you would like it to.

 

[Ken g6]

Just an educated guess, but I suspect you want empty() or is_null().

 

[bhanson]

Validate. Validate. Validate.

$gallery = filter_input(INPUT_POST, 'gallery', FILTER_SANITIZE_URL);
$zip     = filter_input(INPUT_POST, 'zip', FILTER_SANITIZE_NUMBER_INT);

$photos = '';

if (!empty($gallery)) {
    $photos = $gallery;
} else if (!empty($zip)) {
    $photos = "http://www.victorlinphoto.com/photodownloads/$zip";
} else {
    // Default if both are left blank
}

Spoiler

FILTER_SANITIZE_NUMBER_INT isn't completely appropriate since it still allows plus and minus signs but it's pretty close and still gets rid of most extraneous junk. I'm pretty anal and would use a regex but I suspect OP is newish to PHP and there's no use complicating the example with that garbly goop.

[edit]

Spoiler

It will also filter out dashes in zip codes that are full length (and longer than 5 digits), which may or may not be desired. This also takes a dumb filtering approach and assumes the user will enter something remotely resembling a zip code. A better way is to first filter extraneous junk (anything not a dash or digits--this is to help match simple typos) and then match against a regex for a zip code. If it does not match then reload the page with the zip code box highlighted and an easy to read error message. Although, in a modern browser with javascript enabled a user shouldn't get to this point because your client side validation should have already done this, but you have to do it anyway to ensure a good user experience for those <1% of cases.

[edit redux]

Spoiler

As you can see, even something as simple as form validation of a few fields can get really complicated if you dot all of your i's and cross all of your t's. This is why programmers develop their own libraries to do this stuff so you're not redoing the same code over and over. Fortunately some smart guys thought it would be a good idea to have a standardized library and hence Zend Framework was born.

Example of how to validate a zip code using ZF. (code tags in a SPOILER doesn't work)

$validator = new Zend_Validate_PostCode('en_US');
if ($validator->isValid('12345')) {
    // Valid Zip Code!
} else {
    // Invalid Zip Code :(
}

[modificación otra vez]

Spoiler

Although that last example shows how easy it is to use an individual module of ZF, in a real use situation you would be using Zend_Form and would be passing Zend_Validate elements to it to handle validation. Instead of handling your own validation cases with ifs and elses you would let Zend_Form take care of all of that. Instead you would define a form and then add elements to it. You could add a textbox for example that is REQUIRED, and must be a zip code. That's it, then the framework would take care of the rest and handle invalid input appropriately.

 

[Cogman]

Validate. Validate. Validate.

$gallery = filter_input(INPUT_POST, 'gallery', FILTER_SANITIZE_URL);
$zip     = filter_input(INPUT_POST, 'zip', FILTER_SANITIZE_NUMBER_INT);

$photos = '';

if (!empty($gallery)) {
    $photos = $gallery;
} else if (!empty($zip)) {
    $photos = "http://www.victorlinphoto.com/photodownloads/$zip";
} else {
    // Default if both are left blank
}

Spoiler

FILTER_SANITIZE_NUMBER_INT isn't completely appropriate since it still allows plus and minus signs but it's pretty close and still gets rid of most extraneous junk. I'm pretty anal and would use a regex but I suspect OP is newish to PHP and there's no use complicating the example with that garbly goop.

[edit]

Spoiler

It will also filter out dashes in zip codes that are full length (and longer than 5 digits), which may or may not be desired. This also takes a dumb filtering approach and assumes the user will enter something remotely resembling a zip code. A better way is to first filter extraneous junk (anything not a dash or digits--this is to help match simple typos) and then match against a regex for a zip code. If it does not match then reload the page with the zip code box highlighted and an easy to read error message. Although, in a modern browser with javascript enabled a user shouldn't get to this point because your client side validation should have already done this, but you have to do it anyway to ensure a good user experience for those <1&#37; of cases.

[edit redux]

Spoiler

As you can see, even something as simple as form validation of a few fields can get really complicated if you dot all of your i's and cross all of your t's. This is why programmers develop their own libraries to do this stuff so you're not redoing the same code over and over. Fortunately some smart guys thought it would be a good idea to have a standardized library and hence Zend Framework was born.

Example of how to validate a zip code using ZF. (code tags in a SPOILER doesn't work)

$validator = new Zend_Validate_PostCode('en_US');
if ($validator->isValid('12345')) {
    // Valid Zip Code!
} else {
    // Invalid Zip Code :(
}

[modificaci&#243;n otra vez]

Spoiler

Although that last example shows how easy it is to use an individual module of ZF, in a real use situation you would be using Zend_Form and would be passing Zend_Validate elements to it to handle validation. Instead of handling your own validation cases with ifs and elses you would let Zend_Form take care of all of that. Instead you would define a form and then add elements to it. You could add a textbox for example that is REQUIRED, and must be a zip code. That's it, then the framework would take care of the rest and handle invalid input appropriately.

Why did you use all the spoiler tags?

bhanson is right, you really should validate the input, especially if that link is ever stored in a SQL table. Even something as simple as this can lead to a XSS attack and other nasties (php injection anyone?).

The rule of thumb. define what is valid, reject everything else. That generally leads to hard to attack code.

 

[Ken g6]

FYI, reading the original post, "zip" is the name of a zipfile, not a zip code. Still, the validation issue applies. Directly passing a gallery URL should give you pause, as a URL for a site with malicious Javascript could be passed almost as easily.

 

[bhanson]

Why did you use all the spoiler tags?

Sometimes I have an inability to filter the information coming from my brain. I try to keep the examples I provide while teaching simple and to the point, but it is very easy for me to interject a million other things that while may be valuable, can easily cloud the most important parts.

In this case the stuff in the spoilers tag is extraneous and completely secondary to the source code I posted, which may introduce the OP to filter_input() and empty(). If the entire post was uncovered at once it would appear intimidating and long.

 

[Tweak155]

Not 100&#37; sure what you're looking for but empty($_POST['zip']) (this returns true if it is empty) will tell you if anything was put into the zip field.

I'm still wondering if this helps, but you can do an echo based on whats received to send it to the user's page.

So you can do something like:

<?php
if(!empty($_POST['gallery']))
echo "<a href =\"" . $_POST['gallery'] . "\"> TEXT OF LINK </a> ";
else
echo " some other link ";
?>

Basically echo will control whats put to the screen at that point, it is as though HTML was written.