Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
duz
Jul 11, 2005

Come on Ilhan, lets go bag us a shitpost


gwar3k1 posted:

So I should be encapsulating my "action" scripts with the session authorization and not just setting a variable that says the user is logged in and the session is valid? That makes sense.

No, he's saying make sure the page your form submits to authenticates the session as well. That's assuming the script isn't submitting to itself.

Adbot
ADBOT LOVES YOU

KuruMonkey
Jul 23, 2004

gwar3k1 posted:

So I should be encapsulating my "action" scripts with the session authorization and not just setting a variable that says the user is logged in and the session is valid? That makes sense.

So this...
code:
is session
  is same user agent
    is fingerprint
      code
else
  gently caress off
not this...
code:
auth = false
is session
  is same user agent
    is fingerprint
      auth = true
else
  gently caress off
-------------
if auth
  code
else
  do nothing

These are functionally identical, as long as 'gently caress off' stops script execution:

php:
<?
$auth = is_valid_session();

if($auth === FALSE)
{
  header('Location: [url]http://example.com/fuckoff.html[/url]');
  exit;
}

// and now on with the show...
?>
And yes; its FAR more important to check the session on the form handler than the form generator.

If someone gets to a form then gets told to gently caress off, its not the greatest user experience, but its not insecure.

If they are told they can't have the form, but could send a carefully crafted POST or GET and take the action anyway...you have failed at a very fundamental level.

gwar3k1
Jan 10, 2005

Someday soon
Ah okay, so I can have my session validation separate so long as everything that runs behind a login is validated.

php:
<?php
  session_start();
  include('session.php');
  $valid validate_session();

  if($valid)
  {
    //code
  }
  else
  {
    header("location:login.php");
  }
?>

Inverse Icarus
Dec 4, 2003

I run SyncRPG, and produce original, digital content for the Pathfinder RPG, designed from the ground up to be played online.
What's the best way to sanitize input that's going to be stored in a DB?

For example, I want to make sure that someone can't add a post that contains JavaScript. I'd like to allow links and images, but I don't want to bother making a markup language like BBCode or anything like that.

I could add a bunch of RegExes to find these, but invariably someone will break them, and there has to be dozens of things I'm not thinking about. Blacklisting like this might not be the way to go.

Is there an easy way to sanitize the data, or to create a whitelist of "approved" HTML tags?

I'm thinking I could use a RegEx to catch all the tags, and compare them to an array of allowed tags. There has to be something cleaner / faster than this.

supster
Sep 26, 2003

I'M TOO FUCKING STUPID
TO READ A SIMPLE GRAPH

Inverse Icarus posted:

What's the best way to sanitize input that's going to be stored in a DB?

For example, I want to make sure that someone can't add a post that contains JavaScript. I'd like to allow links and images, but I don't want to bother making a markup language like BBCode or anything like that.
Take a look at http://htmlpurifier.org/ or http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/index.php.

Also let me know how it works out - I researched these for a previous project but never ended up using it.

supster fucked around with this message at 23:30 on Dec 21, 2009

gwar3k1
Jan 10, 2005

Someday soon

Inverse Icarus posted:

What's the best way to sanitize input that's going to be stored in a DB?

Probably not comprehensive in any way but PHP has strip_tags which you can add a white list of tags. I don't think it plays well with tags with attributes though.

KuruMonkey
Jul 23, 2004

php:
<?
function hl_ent($t){
// entitity handler
global $C; 
?>
globals are awesome

meaningful names are for noobs

also, look at the entitities on that one :pervert:

(I'm sure it works fine, but drat)

Edit: they have a variable caled $ok2...its an array!

Lumpy
Apr 26, 2002

La! La! La! Laaaa!



College Slice

KuruMonkey posted:

Edit: they have a variable caled $ok2...its an array!

To be fair, the keystrokes you save are well worth the slight loss in clarity of having to type out $okAsWell every time.

supster
Sep 26, 2003

I'M TOO FUCKING STUPID
TO READ A SIMPLE GRAPH

Lumpy posted:

To be fair, the keystrokes you save are well worth the slight loss in clarity of having to type out $okAsWell every time.
Or it could just be named something that is meaningful in context of what the variable represents...

Rat Supremacy
Jul 15, 2007

The custom title is an image and/or line of text that appears below your name in the forums
Yeah I mean basically be boiled down to:

code for the controller that handles the submitted form:
code:

can current user submit this form?
yes:
submit form
no:
do not submit form and call redirect view/template
DIE

Lumpy
Apr 26, 2002

La! La! La! Laaaa!



College Slice

supster posted:

Or it could just be named something that is meaningful in context of what the variable represents...

You don't "get" jokes, do you.

supster
Sep 26, 2003

I'M TOO FUCKING STUPID
TO READ A SIMPLE GRAPH

Lumpy posted:

You don't "get" jokes, do you.
haha. now i do and it's funny :)

Rat Supremacy
Jul 15, 2007

The custom title is an image and/or line of text that appears below your name in the forums
Also

quote:

auth = false
is session
is same user agent
is fingerprint
auth = true

This poo poo should be done by your session class. Your session class should contain all logic required to be trust-able. This means if your method you had pasted in all your scripts turns out to be horribly insecure, you are sorted.

Talking of which, could someone quickly read through and give mine a sanity check?

It is based on the assumption that a quick database query on an active connection is pretty much as quick as a file access (it replaces PHP's $_SESSION):

http://github.com/radiosilence/core/blob/master/session.php

php:
<?php
/**
 * Session management.
 * @package auth
 * @subpackage core
 */
class session
{
    /**
     * Database
     * @var database
     */
    private $db;
    /**
     * Session sid.
     * @var string
     */
    private $session 0;
    /**
     * Session token.
     * @var string
     */
    private $tok 0;
    /**
     * Session user id.
     * @var int
     */
    public $user_id 0;
    /**
     * Session data.
     * @var string
     */
    private $data = array();
    /**
     * Secret phrase.
     * @var string
     */
    private $keyphrase;
    /**
     * Secret phrase to salt passwords.
     * @var string
     */
    private $base_salt;

    /**
     * Starts it all off, gets the sid/tok provided by
     * the cookie, and authorises it/registers it as
     * valid depending on the result.
     * @param database $db database object.
     */
    public function __construct$db )
    {
        require( SITE_PATH "configuration/auth.php" );

        $this->keyphrase     $config_auth'keyphrase' ];
        $this->base_salt     $config_auth'base_salt' ];
        $this->db         $db;
        $sid             $_COOKIE"sid" ];
        $tok            $_COOKIE"tok" ];

        if( isset( $_COOKIE"sid" ] ) && isset( $_COOKIE"tok" ] ) )
        {
            if(DEBUGFB::log"Attempting to load supposed session [" $sid "] ..." );

            # Is it the right tok for sid and IP?
            $sth $db->prepare"
                SELECT     sid, data, user_id
                FROM    sessions
                WHERE    sid     = :sid
                AND    tok     = :tok
                AND    ipv4     = :ipv4
                LIMIT    1
            " );
                
            $e $sth->execute( array(
                ":sid"     => $sid,
                ":tok"     => $tok,
                ":ipv4" => $_SERVER"REMOTE_ADDR" ]
            ));
                
            if( $e )
            {
                $row     $sth->fetch();
                $chall     $this->create_token$sid );
                # would a recreation of this from this host be the same as the real thing?
                if( $chall == $tok )
                {
                    if(DEBUGFB::send"Challenge: "$chall " Real: " $tok"Toks" );

                    $this->set_session( array(
                        "sid"         => $sid,
                        "data"         => json_decode$row"data" ], true ),
                        "tok"         => $tok,
                        "user_id"     => $row"user_id" ] )
                    );
                }

            }
            else
            {
                die( $db->error );
            }
        }
        if( $this->session )
        {
            if(DEBUGFB::log$this"&#10004; Loaded session [" $this->session "]" );
        }
        else
        {
            if(DEBUGFB::log"× Session not loaded because it doesn't exist." );
        }

    }

    public function __destruct()
    {

        if( $this->session )
        {
            if(DEBUGFB::log$this"Saving session [" $this->session "] ... " );
            $db $this->db;
            
            $sth $db->prepare"
                UPDATE    sessions
                SET    data     = :data
                WHERE    sid    = :sid
                LIMIT    1"
            );
            
            $sth->execute( array( ":data" =>  json_encode$this->data ), ":sid" => $this->session ));
            if( $e )
            {
                if(DEBUGFB::log"&#10004; Saved session." );
            }
            else
            {
                if(DEBUGFB::error"× Could not write session." );
            }

        }
        else
        {
            if(DEBUGFB::log"× Not destroying session because it doesn't exist." );
        }

    }

    /**
     * Gets stuff from data, overloader.
     * @param $prop_name Property
     * @param $prop_value Property data
     * @return boolean
     */
    function __get$prop_name )
    {

        if ( isset( $this->data$prop_name ] ) )
        {
            return $this->data$prop_name ];

        }
        else
        {
            return false;
        }

    }

    /**
     * Sets stuff to data, overloader.
     * @param $prop_name Property
     * @param $prop_value Property data
     * @return boolean
     */
    function __set$prop_name$prop_value )
    {
        $this->data$prop_name ] = $prop_value;
        return true;
    }

    /**
     * Creates a session, puts it in the database,
     * returns the ID.. Assumes login has succeeded.
     * @param integer $user_id User ID
     * @param string $passhash Hashed password.
     * @param string $email User's email.
     * @return array Either a fail or an array with $sid, $id, and $tok.
     */
    public function create_session$user_id )
    {
        $sid     $this->create_sid();
        $tok     $this->create_token$sid );
        $db    $this->db;

        $sth $db->prepare"
            DELETE
            FROM    sessions
            WHERE    user_id = :user_id
            AND    ipv4    = :ipv4"
        );
        $sth->execute( array( "user_id" => $user_id"ipv4" => $_SERVER"REMOTE_ADDR" ] ));


        $sth2 $db->prepare"
            INSERT INTO sessions (
                sid, tok, ipv4, user_id
            )
            VALUES (
                :sid, :tok, :ipv4, :user_id
            )
        " );
        
        $res $sth2->execute( array(
            ":sid"         => $sid,
            ":tok"        => $tok,
            ":ipv4"        => $_SERVER"REMOTE_ADDR" ],
            ":user_id"    => $user_id 
        ));
        
        if( $res )
        {
            $return = array( "sid" => $sid"id" => $user_id"tok" => $tok );
        }
        else
        {
            $return 0;
        }
        
        return $return;
    }

    /**
     * Destroys the session, deletes from DB, unsets cookies.
     * 
     */
    public function destroy_session()
    {
        $db $this->db;
        $sth $db->prepare"
            DELETE FROM    sessions
            WHERE        sid     = :sid
            AND        ipv4     = :ipv4
            AND        tok     = :tok
        " );
        
        $sth->execute( array(
            ":sid"    => $this->session,
            ":ipv4"    => $_SERVER"REMOTE_ADDR" ]
        ));
        
        setcookie"sid""DEAD"time()-1WWW_PATH "/"nullfalsetrue );
        setcookie"tok""DEAD"time()-1WWW_PATH "/"nullfalsetrue );
        return 1;
    }
         
    /**
     * Makes a hash from a password string.
     * @param string $password unhashed password
     * @return string password hash
     */
    public function password_hash$password$salt )
    {
        $hash hash"sha256"$password sha1$salt $this->base_salt ) );
        return $hash;
    }

    /**
     * Sets the object's session to the right things.
     * @param hash $sid
     * @param integer $id
     */
    public function set_session$s )
    {
        if(DEBUGFB::send$s"Setting Session" );
        $this->session     $s'sid'  ];
        $this->user_id     $s'user_id'  ];
        $this->data    $s'data' ];
        $this->tok    $s'tok'  ];
    }

    /**
     * Sets the cookies, with httponly.
     * @param hash $tok
     */
    public function set_cookie()
    {
        setcookie"sid"$this->sessiontime()+(3600*24*65), WWW_PATH "/"nullfalsetrue );
        setcookie"tok"$this->toktime()+(3600*24*65), WWW_PATH "/"nullfalsetrue );
        if(DEBUGFB::log("Setting up cookies.");
    }

    /**
     * Generates a new auth token based on session ID.
     * @param string $passhash Password hash.
     * @param string $email User's email.
     */
    private function create_token$sid )
    {
        # Token generation code.
        $hash sha1$this->keyphrase $_SERVER'REMOTE_ADDR' ] . $sid );
        return $hash;
    }

    /**
     * Generate a simple sid hash.
     * @return hash sid
     */
    private function create_sid()
    {
        return sha1microtime() . $_SERVER'REMOTE_ADDR' ]);
    }
}
?>
Please, rip me to shreds, and ignore the horrible tabbing the forums give it.

Things to consider:
- Should I use User Agent authentication? It just seems like a very small road hump in the face of anyone malicious.
- If the script ends prematurely, the session isn't saved. This seems like a bad thing, but I actually think it is more secure, as it relies on PHP ending properly.
- Perhaps one day I will use camelCase. Today is_not_the_day.

Rat Supremacy fucked around with this message at 03:44 on Dec 22, 2009

MrMoo
Sep 14, 2000

It's a bit messy with proxies, it's generally not a good idea to use the IP if you would like anyone in a big company to access such a site.

You could put a LIMIT on the DELETE FROM SQL.

MrMoo fucked around with this message at 03:59 on Dec 22, 2009

DarkLotus
Sep 30, 2001

Lithium Hosting
Personal, Reseller & VPS Hosting
30-day no risk Free Trial &
90-days Money Back Guarantee!
For a good sessions management class, take a look at this:
Database Sessions Management Class

Rat Supremacy
Jul 15, 2007

The custom title is an image and/or line of text that appears below your name in the forums

MrMoo posted:

It's a bit messy with proxies, it's generally not a good idea to use the IP if you would like anyone in a big company to access such a site.

You could put a LIMIT on the DELETE FROM SQL.

LIMIT was intentionally removed - a handy way of getting rid of duped sessions if they exist for some reason - sids should be unique.

I don't really know what other ways there are really to verify that a host is actually a host other than IP. The way it is done in theory allows more than one session per IP, so shared IPs shouldn't matter.

Still need a mechanism for cleaning up orphan sessions, perhaps some sort of cleaning batch run nightly, and a field for expires or something.

MrMoo
Sep 14, 2000

haywire posted:

LIMIT was intentionally removed - a handy way of getting rid of duped sessions if they exist for some reason - sids should be unique.

Useful when deleting all existing sessions in create_session() but not so when deleting a single session in destroy_session(), although you might want to allow multiple sessions anyway.

REMOTE_ADDR is the address of the last proxy a client is using, if they have multiple proxies the session might bounce between different IP addresses. HTTP_X_FORWARDED_FOR might be set to the clients actual address, or even 127.0.0.1 for various reasons (security through obscurity is popular). With a Squid & HAVP sandwich it can end up a list of addresses, mine shows: "10.6.15.69, 127.0.0.1".

Rat Supremacy
Jul 15, 2007

The custom title is an image and/or line of text that appears below your name in the forums

MrMoo posted:

Useful when deleting all existing sessions in create_session() but not so when deleting a single session in destroy_session(), although you might want to allow multiple sessions anyway.

REMOTE_ADDR is the address of the last proxy a client is using, if they have multiple proxies the session might bounce between different IP addresses. HTTP_X_FORWARDED_FOR might be set to the clients actual address, or even 127.0.0.1 for various reasons (security through obscurity is popular). With a Squid & HAVP sandwich it can end up a list of addresses, mine shows: "10.6.15.69, 127.0.0.1".

So which would you recommend using? HTTP_X_FORWARDED_FOR unless it == 127.0.0.1?

Atimo
Feb 21, 2007
Lurking since '03
Fun Shoe
I am normally in the .Net area, but a coworker/boss asked me to take a look at a side project of theirs and I'd love a bit of advice on how to take care of this issue for them, or at least tell them how they should proceed.

They have a combobox with a text box above it serving as a javascript filter. The problem though, is that it is REALLY slow. The first time I loaded it up the page took 40 seconds or so to load, and typing into the filter can take 10+ seconds per key stroke.

A quick look at the source showed why - it was returning 1.25meg response stream. Over 20,000 lines of javascript generating by a peice of php. Heres a snipped sample;

PHP Code:
code:
function rse_lfilter(string,object)
	{
		var locatii = new Array();
		var ids = new Array();
		object.length = 1;
		<?php foreach($this->locationsList as $obj)
		{	?>
			locatii.push('<?php echo $db->getEscaped($obj->LocationName,true); ?>');
			ids.push('<?php echo $obj->IdLocation; ?>');
		<?php
		}
		?>
		var j = 1;
		for(var i=0;i<locatii.length;i++)
			if (locatii[i].toLowerCase().indexOf(string.toLowerCase()) != -1)
			{
				object.options[j] = new Option(locatii[i],ids[i]);
				j++;
			}
	}
Rendered java script:
code:
	function rse_lfilter(string,object)
	{
		var locatii = new Array();
		var ids = new Array();
		object.length = 1;
					locatii.push('Location Needed');
			ids.push('2');
					locatii.push('Zydeco');
			ids.push('5');
					locatii.push('Zumanity Theatre at New York New York Hotel and Casino');
			ids.push('6');
					locatii.push('Zooll');
			ids.push('7');

----- snip 20,000 lines of this

var j = 1;
		for(var i=0;i<locatii.length;i++)
			if (locatii[i].toLowerCase().indexOf(string.toLowerCase()) != -1)
			{
				object.options[j] = new Option(locatii[i],ids[i]);
				j++;
			}
	}
The combobox then goes on to have 10,720 html items.


code:
<input type="text" name="lfilter" id="lfilter" value="" size="15" onkeyup="rse_lfilter(this.value,document.getElementById('IdLocation'));" />	

<select name="IdLocation" id="IdLocation" class="inputbox" size="5">
<option value="0"  selected="selected">- Select Location -</option>

--- snip 10,000 items

</select>
As a bandaid I suggested changing the filter from doing an indexOf call on all 10,000 items to a substring(0,search string length) == direct comparison, which might help a little:

code:
function rse_lfilter(string,object)
    {
        var locatii = new Array();
        var ids = new Array();
        object.length = 1;
        <?php foreach($this->locationsList as $obj)
        {    ?>
            locatii.push('<?php echo $db->getEscaped($obj->LocationName,true); ?>');
            ids.push('<?php echo $obj->IdLocation; ?>');
        <?php
        }
        ?>
        var j = 1;

string = string.toLowerCase();

        for(var i=0;i<locatii.length;i++)
            
// faster then indexOf comparisons, enough to make a differnce?
if (locatii[i].toLowerCase().substring(0,string.length) == string)


            {
                object.options[j] = new Option(locatii[i],ids[i]);
                j++;
            }
    }
I think the whole page needs to be reworked, but not being a php expert I wouldn't know where to start.

Any thoughts guys?

Tad Naff
Jul 8, 2004

I told you you'd be sorry buying an emoticon, but no, you were hung over. Well look at you now. It's not catching on at all!
:backtowork:
Yeah. As far as I can tell it's trying to do an AJAXy autosuggest, but without the AJAX. Without the AJAX you're stuck with 20000 lines of PHP-generated Javascript, so page load times will suck no matter what. The JS hunting through each element doing an indexOf is just icing on the cake. A more intelligent method would be to make a simple PHP script accepting a parameter (call it $fragment after sanitizing etc) to output the contents of the <select> based on a database query like "SELECT location_name, location_code FROM locations WHERE location_name LIKE '%$fragment%' LIMIT 100". And only have it kick in after three or so keystrokes. The the onkeyup event can just call a function that will replace the content of the <select> with whatever the server says: onkeyup="populate_select_with_suggestions(selectId,this.value)" or something like that.

Fluue
Jan 2, 2008
I'm doing a little bit of hobbyist scripting. I'm currently trying to make a random Pokemon generator where a user selects the number of Pokemon they want from a drop-down form and then it generates however many Pokemon they chose (taking the data from a MySQL database).

However, I want to add a second part to the script where it can limit the random Pokemon to certain types (ex: 5 pokemon, all grass).

I've gotten this script so far:

http://pastebin.com/dd935973

When I run it I get this error:

code:
Warning: mysqli_stmt_fetch() expects parameter 1 to be mysqli_stmt, boolean ...
I'm not sure where mysqli_stmt_fetch is taking a wrong turn. Any suggestions?

Rat Supremacy
Jul 15, 2007

The custom title is an image and/or line of text that appears below your name in the forums
You are doing the fetch outside the if block. So the prepare is failing for which whatever reason and returning false, but because the fetch is outside of that if block, it is still trying to do fetch on $stmt, which is boolean false as opposed to a statement. In short: Your prepare is failing.

A quick word of advice, avoid procedural style (definitely) and mysqli (ideally) altogether and use PDO as an object. It is much cleaner and nicer.

You could rewrite:

php:
<?
    if ($stmt = mysqli_prepare($link, "SELECT * FROM pokemon WHERE type =?")) {
        mysqli_stmt_bind_param($stmt, 's', $type);
        mysqli_stmt_bind_result($stmt, $id, $name);
        mysqli_stmt_execute($stmt);
        mysqli_stmt_store_result($stmt);
                }
$rows = array();
while(mysqli_stmt_fetch($stmt)) {
$rows[] = array($id, $name);
}
print_r( $rows[rand()] );
?>
Into something happy and pretty and nice like:

php:
<?

// Somewhere you will have initiated $db as $pdo = new PDO( "mysql:dbname=testdb;host=127.0.0.1;port=3600", "user", "pass" ); instead of doing mysqli_connect or whatever.

$stmt = $db->prepare( "
    SELECT *
    FROM pokemon
    WHERE type = :type
" );

$stmt->execute( array(
    ":type" => $type
));

if( $rows = $stmt->fetchAll() )
{
    print_r( $rows[ rand() ] );
}
else
{
    die( "Bugger.\n" );
}
?>

Rat Supremacy fucked around with this message at 20:25 on Jan 2, 2010

Fluue
Jan 2, 2008
Thanks, Haywire.

I put in the new PDO object and tried to redo the db connection like so:
php:
<?
$db = new PDO( "mysql:dbname=fluue_pokemon;host=localhost;port=3600", "fluue_*****", "***removed***" );
?>
and put the PDO like so:

php:
<?
do
{
    $i++;
    
    

$stmt = $db->prepare( "
    SELECT *
    FROM pokemon
    WHERE type = :type
" );

$stmt->execute( array(
    ":type" => $type
));

if( $rows = $stmt->fetchAll() )
{
    print_r( $rows[ rand() ] );
}
else
{
    die( "Bugger.\n" );
}


}
while($i<=$limit);
?>
But I'm still getting the "Bugger" error message. Did I connect to my db incorrectly?

Peanut and the Gang
Aug 24, 2009

by exmarx
Don't put the colon in the "type" array key. The colon in the prepare statement indicates that the corresponding variable name, sans the colon, will be passed to the execute method.

php:
<?
$stmt->execute( array(
    "type" => $type
));
?>
EDIT: This is actually wrong; you can use it with or without the colon. My mistake.

Peanut and the Gang fucked around with this message at 19:56 on Jan 3, 2010

Fluue
Jan 2, 2008
Yeah, I wasn't sure what that colon was there for...

However, I'm still getting the die error. It doesn't seem to be fetching the rows, though I'm not 100% sure that's the problem.

edit:
http://pastebin.com/m2f5d418d

Here's what I've got now, in case anyone needs to see the updated code.

Peanut and the Gang
Aug 24, 2009

by exmarx
Check out the error log. And run var_dump() or print_r() a lot.

As in, wrap it around each statement and look for ones that output bool(false):

php:
<?
var_dump($db = new PDO( "mysql:host=localhost;dbname=fluue_pokemon", "fluue_****", "****" ));

//...

var_dump($stmt = $db->prepare( "
   SELECT *
   FROM pokemon
   WHERE type = :type
" ));
 
var_dump($stmt->execute( array(
    "type" => $type
)));
?>

Fluue
Jan 2, 2008
code:
object(PDO)#1 (0) { } object(PDOStatement)#2 (1) { ["queryString"]=>  string(54) " SELECT * FROM pokemon WHERE type = :type " } bool(false) array(0) { } Bugger. 
It appears to be coming from the execute command, which puts 0 into the array. PDO #1 is my db connection. Does the 0 mean it's not connecting?

php:
<?
$stmt->execute( array(
    "type" => $type
))
?>

Peanut and the Gang
Aug 24, 2009

by exmarx
That looks like the actually SELECT statement is wrong, as if the "pokemon" table or the "type" column didn't exist.

Add this after the execute() call:
var_dump($stmt->errorInfo());

Fluue
Jan 2, 2008
Gah, it turns out I was calling the wrong column. Had to be "type1" instead of "type".

It now returns the bool as "true," but I'm still get a die error at the end.

code:
object(PDO)#1 (0) { } object(PDOStatement)#2 (1) { ["queryString"]=>  string(56) " SELECT * FROM pokemon WHERE type1 = :type1 " } bool(true) array(1) { [0]=>  string(5) "00000" } array(0) { } Bugger. 

Peanut and the Gang
Aug 24, 2009

by exmarx
My guess is now none of the the rows match the WHERE clause, so it's giving a blank array.

btw, when doing var_dump(), "array(0) { }" means an empty array. Syntax is "array(numberOfElements) {name=>value, ...}".

Fluue
Jan 2, 2008
Which is odd, because type1 exists in the table and I know for a fact that many of the entries -should- match the WHERE clause. Even setting it to Any where it makes $type = "*" gives me an empty array.

Peanut and the Gang
Aug 24, 2009

by exmarx
That's actually looking for the string "*"; it doesn't act like a wildcard here.
But you can use the LIKE keyword, which sees '%' as a wildcard.

So on case 0, set $type='%';

Then edit the WHERE clause:
WHERE type1 LIKE :type1

Fluue
Jan 2, 2008
I think that fixed something: the execute query grabs information from all the columns which is definitely not good...

I'm thinking this project needs to be redone from scratch. The array doesn't seem to want to pick-up any information sent to it.

DoctorScurvy
Nov 11, 2005
More of a passing curiosity, really
See maybe this is why I'm not a professional developer, but I don't see why you need all of that class stuff when this will do the same thing:
php:
<?php
// list of acceptable pokemon type IDs
$poketypes = array(0=>'%'1=>'grass'2=>'fire'3=>'ground');

// get the type and number to select
$ptype = (int) $_POST['t1'];
$pokemonlimit abs(intval($_POST['pokemonlimit']));

// if the number is a chosable option, return its database value, otherwise select 'all types' by default
$poktype = (isset($poketypes[$ptype])) ? $poketypes[$ptype] : '%';

// run query and return results
$query mysql_query("SELECT * FROM pokemon WHERE `type1` LIKE '".$poktype."' ORDER BY rand() LIMIT ".$pokemonlimit);
while ($data mysql_fetch_assoc($query)) {
    echo $data['name'].'<br>';
}
?>
(edited a few times with minor improvements)

DoctorScurvy fucked around with this message at 04:16 on Jan 3, 2010

Fluue
Jan 2, 2008
Your code makes more sense, but it seems to be missing a some kind of closing here:
php:
<?
$query = mysql_query("SELECT * FROM pokemon WHERE `type` LIKE '".$poktype."' ORDER BY rand() LIMIT ".$pokemonlimit);
while ($data = myql_fetch_assoc($query)) {
    echo $data['name'].'<br>';
}
?>
I can't seem to figure it out with my IDE either.

DoctorScurvy
Nov 11, 2005
More of a passing curiosity, really

Fluue posted:

Your code makes more sense, but it seems to be missing a some kind of closing here:
I accidentally had myql_fetch_assoc instead of mysql_fetch_assoc. Fixed in my original code.

Fluue
Jan 2, 2008
The script works great except for one thing: it doesn't seem to want to take the type into account in the WHERE clause. I'll keep trying out different things with your code, though.

DoctorScurvy
Nov 11, 2005
More of a passing curiosity, really

Fluue posted:

The script works great except for one thing: it doesn't seem to want to take the type into account in the WHERE clause. I'll keep trying out different things with your code, though.
Remember that you changed it to type1 so you'll need to update the query for that. If it still isn't limiting the results, then try making the script print out all of the variables at various points to make sure they are entering properly - most importantly, check to make sure $_POST['t1'] is getting in, because if the script doesn't see it (ie, your form method is not POST), then it will default to 'show all' % and the limit will be 0.

Fluue
Jan 2, 2008
Thank you! I seem to have solved the problem. I rechecked all my $_POST []; variables and then saw that I had named my form. I removed the form name and now everything seems to be in working order.

Thank you Haywire, Barbarianbob, and DoctorScurvy for helping me work through this script!

Adbot
ADBOT LOVES YOU

KuruMonkey
Jul 23, 2004

DoctorScurvy posted:

See maybe this is why I'm not a professional developer, but I don't see why you need all of that class stuff when this will do the same thing:


There are a few inter-related things here, but in general there is no inherent advantage to well designed/structured/written OO code as opposed to equally well designed/structured/written procedural code.

Some people do make the mistake of thinking that the OO paradigm in some way 'inherently' produces better designed/structured/written code. They are incorrect; it usually means either that they themselves just better understand how to do OO well than how to do procedural structured code well, or that the ''OO way of thinking' better matches their natural conceptualising of problems.

In the specific 'why use PDO?' case, its the fact that it protects your DB from SQL injection. Don't get me wrong, your script was protected as well, but there are drawbacks to your method, particularly regarding URLs and extensibility. Lets take a contrived pokemon example:

We'll start with a state where there are 'water' and 'earth' pokemons (bear with me; I was never a pokefan, OK?)

In your system, we'd have type 0 as any, type 1 as water and type 2 as earth. Our request URL for water pokemon might be:

http://www.example.com/pokemon/bytype/1

or something, presuming we want some nice readable urls.

If someone goes for
http://www.example.com/pokemon/bytype/99

then they get all the pokemons - as an aside; this is probably not the desired result in that case, as there are in fact NO type 99 pokemons

In your code you'll have, as you had,
php:
<?
$valid_types = array(0=>'%', 1=>'water', 2=>'earth');
// plus the pul from POST and lookup code
?>
If we use PDO, the value for 'type' is sanitised before being sent to the DB, this frees us up to send the ACTUAL POST VALUE to the DB in the query:
php:
<?
// this is pseudocode...
$query = $db->prepare('SELECT name FROM pokemon WHERE type = :type');
$query->execute(array('type'=>$_POST['type']);
?>
Assuming some mod_rewrite trickery, we can use URLs like:
http://www.example.com/pokemon/bytype/water

and if you send
http://www.example.com/pokemon/bytype/madeuptype

you will get 'no pokemons of type "madeuptype"' because there aren't any in the DB. Which, as I said, is actually a more sensible result when an invalid type is given than, conceptually; all pokemon are of this invalid type - which is what returning all by default is saying.

Note that the URLs for the PDO version ore more meaningful than the /bytype/1 version. Its a small thing, but quite nice in the general scheme of things. Nobody should need to track whether type 63 or type 49 pokemons are the 'cheese' ones...

Secondly, and more importantly, six months down the line, they release some lovely new 'wood' pokemon, and you enter them in the DB.

Then you can make your links as appropriate, update forms etc.

In the mysql_XYZ version, you need to update your array and assign a new pokemon type number to them.

In the PDO version, once the DB is updated, and the links / forms are updated the work is DONE, because the actual type string is what drives the code, by adding the data, the existing SQL will look them up just fine.

Incidentally, when you don't manually interject an id number to type value lookup, the links and forms can conceivably update themselves, by being built to lookup 'SELECT DISTINCT type FROM pokemon' etc

Getting the forms and links that give users access to the data to be driven BY the data is where the magic happens, basically. Its all about using the tool that allows you to do the least work.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply