query string in php

Question

This code have some problem of Security? is a correct way to make a query string ?

I don't usually use this type of code, so my question.

 <?php
    $id = $_REQUEST['id'];
    ?>
    <?
    switch($id) {

        case "principal":
            $pagina = "pag.php";
            break;
        case "perfil":
            $pagina = "secure.php";
            break;

        default:
            $pagina = "home.php";
            break;
    }
    ?>

    <?
    if( (isset($pagina)) and (file_exists($pagina)) ) {
        include($pagina);
    } else {
        echo "Page error";
    }
    ?>

require_once is much better practise instead of include and try to use intval if it possible (in your ids) — e382df99a7950919789725ceeec126
– e382df99a7950919789725ceeec126, Commented May 5, 2011 at 14:42
disagree with require_once, it takes a little longer than require and if your app is organized correctly you shouldnt need it. — Galen
– Galen, Commented May 5, 2011 at 15:02

Galen · Accepted Answer · 2011-05-05 15:07:25Z

1

If your select statement gets big i'd recommend using arrays:

// array of pages
$paginas = array(
    'principal' => 'pag.php',
    'perfil' => 'secure.php'
);

$id = (int)$_GET['id'];

if ( isset( $paginas[$id] ) && file_exists($paginas[$id]) ) {
    require( $paginas[$id] );
}
else {
    require('home.php');
}

answered May 5, 2011 at 15:07

Galen

30.2k9 gold badges75 silver badges90 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

bpeterson76 · Accepted Answer · 2011-05-05 14:40:37Z

1

The only possible problem is that if you're getting ID from the $_REQUEST, someone could change your query by putting ?id=x in the URL string. It might not matter in your case (tough to say since we don't know the full context of what's going on) but it's a possibility. You could make it a bit more secure by sending the variable to the page via a $_POST, which is hidden to the user.

answered May 5, 2011 at 14:40

bpeterson76

12.9k6 gold badges51 silver badges82 bronze badges

Comments

Syjin · Accepted Answer · 2011-05-05 14:43:47Z

1

An intruder could guess the id from secure.php and access it by entering yourpage.php?id=perfil in his browser...

answered May 5, 2011 at 14:43

Syjin

2,8091 gold badge30 silver badges31 bronze badges

1 Comment

anvd Over a year ago

if the page is private i will use sessions

Mike Thomsen · Accepted Answer · 2011-05-05 14:40:28Z

0

You have a handler for every situation involving $id so it should be safe. You also don't need this since $pagina will always be set: (isset($pagina))

answered May 5, 2011 at 14:40

Mike Thomsen

37.7k11 gold badges63 silver badges86 bronze badges

Comments

James · Accepted Answer · 2011-05-05 14:40:53Z

0

I'd add some htmlspecialchars to that. You'll never know what people could try doing!

If you wanted to be "doubley" sure, then you could always do;

filter_var($val, FILTER_SANITIZE_STRING);

answered May 5, 2011 at 14:40

James

5,1697 gold badges43 silver badges80 bronze badges

5 Comments

Michael J.V. Over a year ago

Why would you add htmlspecialchars? It makes no sense in the context of what is being done.

James Over a year ago

This is true and be getting over zealous with it all. Still, filter_var is always good.

Michael J.V. Over a year ago

You are familiar with KISS principle? :)

tplaner Over a year ago

neither of those are necessary and while they might seem like a good practice they waste CPU time (which doesn't matter for small sites, but if it gets large it could) -- you can't break out or inject code into a switch statement expression.

James Over a year ago

In this case, I agree it's not necessary since he covers his bases with his switch statement.

Collectives™ on Stack Overflow

query string in php

5 Answers 5

Comments

Comments

1 Comment

Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

1 Comment

Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related