[PHP PDO / Mysql] Order by

[Oid]

Ik heb een simpele query en wil daar in order by doen via PDO

nu heb ik hier http://stackoverflow.com/...ng-prepared-pdo-statement gelezen dat het niet mogelijk is via prepared statement. nu vraag ik me af wat is wel de juiste manier?

hier komt het op neer:

PHP:

$orderByField = 'field1 ASC';
if(isset($_GET['orderByField']) && !empty($_GET['orderByField'])){
     $orderBy = $_GET['orderByField'] . ' ' $_GET['orderBy'];
}
$query = 'SELECT * FROM table WHERE a = b ORDER BY :order';
$statement->prepare($query); //$statement is pdo class
$statement->bindValue(':order', $orderBy);
//de rest

echter werkt dit niet dus dacht ik het zo:

PHP:

$orderByField = 'field1 ASC';
if(isset($_GET['orderByField']) && !empty($_GET['orderByField'])){
     $orderBy = $_GET['orderByField'] . ' ' $_GET['orderBy'];
}
$query = 'SELECT * FROM table WHERE a = b ORDER BY ' . mysql_real_escape_string($orderBy) . '';
$statement->prepare($query); //$statement is pdo class
//de rest

Dit resulteert in:

code:

 mysql_real_escape_string() [<a href='function.mysql-real-escape-string'>function.mysql-real-escape-string</a>]: Access denied for user 'ODBC'@'localhost' (using password: NO)

Hoe moet dit wel, of moet ik me geen zorgen maken (ondanks dat het USER_INPUT is) en kan er niks fout gaan?

het is voorbeeldcode

[Voor 2% gewijzigd door Oid op 17-06-2011 14:19. Reden: Verkeerde URL]

[RobIII]

Oid schreef op vrijdag 17 juni 2011 @ 14:18:
of moet ik me geen zorgen maken (ondanks dat het USER_INPUT is) en kan er niks fout gaan?

Wat denk je zelf Natuurlijk gaat dit fout als 't userinput is; het wordt niet op magische wijze (pun intended) voor je escaped hoor

2 seconden googlen gaf me echer dit.
/edit: hoewel ik nu zie dat 'ie ook quotes voor/achter de escaped text zet... sjees, lekker consistent guys

[Voor 10% gewijzigd door RobIII op 17-06-2011 14:29]

[Mercatres]

mysql_real_escape_string hoort bij de mysql_library, die werkt niet met PDO. Met prepare() doet hij dat al min of meer. PDO is ook alleen echt interessant als je de parameters gaat binden, maar dat doe je al

[Oid]

Zodra ik PDO::quote gebruik werkt de order by niet?

[RobIII]

Oid schreef op vrijdag 17 juni 2011 @ 14:29:
Zodra ik PDO::quote gebruik werkt de order by niet?

RobIII schreef op vrijdag 17 juni 2011 @ 14:22:

/edit: hoewel ik nu zie dat 'ie ook quotes voor/achter de escaped text zet... sjees, lekker consistent guys

In effect zal dat dus dit geven:

SQL:

select foo, bar from foobar where x = y order by 'z'

En die quotes om "z" wil je dus niet. Ik zou overigens sowieso backticks gebruiken:

SQL:

select `foo`, `bar` from `foobar` where `x` = `y` order by `z`

Even aangenomen dat je in je veldnamen enkel a-z en underscores gebruikt (eventueel nog cijfers) dan kun je natuurlijk makkelijk met een regexje even de veldnaam checken op meuk:

PHP:

if ($orderByField != preg_replace('/[^a-z0-9_]/i','', $orderByField))
  throw new Exception('Invalid fieldname!');

De Asc/Desc hoort natuurlijk sowieso niet in een variabele met de naam $orderbyField maar in een aparte variabele of ternary

[Voor 36% gewijzigd door RobIII op 17-06-2011 14:37]

[Oid]

En dan denk ik dat het makkelijkst is dat je "z" zelf even checkt voor je 'm in de query mikkert.

Daar dacht ik zelf ook al aan, maar vind het wel apart dat zoiets niet geimplementeerd zit binnen die library.

Waarom raad je me eigenlijk aan om backticks te gebruiken?

Maar ik weet voldoende wat betreft de order in PDO, bedankt.

[RobIII]

Oid schreef op vrijdag 17 juni 2011 @ 14:34:
maar vind het wel apart dat zoiets niet geimplementeerd zit binnen die library.

Omdat 't hele idee nou net is dat je niet meer zelf querystrings in elkaar gaat liggen prutsen maar query parameters gebruikt

Oid schreef op vrijdag 17 juni 2011 @ 14:34:
Waarom raad je me eigenlijk aan om backticks te gebruiken?

Dat is, wmb althans, altijd een goed idee. Best practise if you will. Als je nou een veld met de naam "status" hebt; en in MySQL 7.12.4 blijkt dat opeens een reserved word te zijn geworden (fat chance, maar je weet nooit) dan hoef je je code van niet voor tot achter na te lopen. Een leuk (praktijk) voorbeeld is het 'action' keyword. Als je kijkt naar 5.0, 5.1, 5.5 en 5.6 dan zie je er elke keer weer een paar nieuwe reserved words bij komen.

[Voor 41% gewijzigd door RobIII op 17-06-2011 14:43]

[Oid]

RobIII schreef op vrijdag 17 juni 2011 @ 14:38:
[...]

Omdat 't hele idee nou net is dat je niet meer zelf querystrings in elkaar gaat liggen prutsen maar query parameters gebruikt

Ja dat wil ik ook, maar voor ORDER BY werkt dat dus niet.

[...]


Dat is, wmb althans, altijd een goed idee. Best practise if you will. Als je nou een veld met de naam "status" hebt; en in MySQL 7.12.4 blijkt dat opeens een reserved word te zijn geworden (fat chance, maar je weet nooit) dan hoef je je code van niet voor tot achter na te lopen. Een leuk (praktijk) voorbeeld is het 'action' keyword

http://www.pfz.nl/wiki/backticks/ had ik juist anders gelezen, maar ik begrijp zeker wat je bedoelt.

Bedankt voor je tijd!

[RobIII]

Oid schreef op vrijdag 17 juni 2011 @ 14:43:
http://www.pfz.nl/wiki/backticks/ had ik juist anders gelezen, maar ik begrijp zeker wat je bedoelt.

quote: http://www.pfz.nl/wiki/backticks/

De bovenstaande code zal niet werken, omdat de client code vergeten was om backticks te gebruiken

En dat is de schuld van wie? Juist, de client!

quote: http://www.pfz.nl/wiki/backticks/

Door ze in het totaal niet te gebruiken, kan je deze problemen ten alle tijden voorkomen.

Met 'ze' doelen ze hier op de de reserved words; niet de backticks.

Weer zo'n "kwaliteitsartikel"

Anyhoe: Als je kunt voorkomen om reserved words te gebruiken dan moet je dat vooral doen, maar als iets een 'group' heet dan dikke_lul_3_bier en noem ik het veld gewoon group. Zoals je in mijn vorige post zag komen er met regelmaat nieuwe reserved words bij; hoe weet jij of 'foo' in MySQL 9.3 niet ook reserved is?

Dat je 'delete' of 'drop' niet als veldnaam gebruikt vind ik dan nog enigszins te verdedigen, maar feitelijk geldt ook voor die hetzelfde.

Als je _altijd_ backticks (of blokhaken voor MSSQL of blabla voor database_X enz... _{fijn, standaardisatie} ) gebruikt gaat 't gewoon altijd goed

[Voor 38% gewijzigd door RobIII op 17-06-2011 14:53]

[ameesters]

en daarbij sluit ik mij volledig aan bij rob! ik kan niet genoeg benadrukken hoe belangrijk dat is.
Het bespaard achteraf veel tijd en moeite, ikzelf heb nooit de fout gemaakt maar ben het vaak genoeg tegen gekomen!

[Avalaxy]

RobIII schreef op vrijdag 17 juni 2011 @ 14:31:

In effect zal dat dus dit geven:

SQL:

select foo, bar from foobar where x = y order by 'z'

En die quotes om "z" wil je dus niet. Ik zou overigens sowieso backticks gebruiken:

SQL:

select `foo`, `bar` from `foobar` where `x` = `y` order by `z`

Volgens mij klopt dit niet helemaal. Je gebruikt in MySQL toch backticks voor kolomnamen, tabelnamen, etc. en voor waardes gewone quotes ( ' )?

[Voutloos]

Dat klopt en dat zegt Rob ook, want er staan enkel kolomnamen.

[Avalaxy]

Oh ok, ik dacht dat hij ook waardes bedoelde Verkeerd gelezen

[Down]

Een order by op een waarde zou ook vreemd zijn

Het zal overigens een kwestie van gewenning zijn, maar ik vind de backtic verwarrend, simpelweg omdat hij op de single quote lijkt, maar ik werk dan ook nooit met MySQL. Wat dat betreft vind ik de blokhaken van SQL Server wel fijn (ondanks dat ik ze zelf weinig typ dankzij de OR mapper).

En wij hebben in productie ook keywords (group, order), ik denk dat deze toch redelijk vaak voorkomem

[MueR]

Ik zou overigens ook nooit een order by toevoegen op basis van directe user input. Dat zou altijd langs een array met door mij gedefineerde kolommen gaan. Indien de waarde niet aanwezig in die kolom, dan maar naar de default. Idem met iets simpels als ASC en DESC.

[Oid]

MueR schreef op vrijdag 17 juni 2011 @ 23:57:
Ik zou overigens ook nooit een order by toevoegen op basis van directe user input. Dat zou altijd langs een array met door mij gedefineerde kolommen gaan. Indien de waarde niet aanwezig in die kolom, dan maar naar de default. Idem met iets simpels als ASC en DESC.

Zo heb ik het nu ook gebouwd.

Waarom via user_input:

http://datatables.net/rel...ver_side/server_side.html gebruik ik voor de overzichten van data, de sortering is gebasseerd op $_GET

Iedereen bedankt voor de tips!

[Voor 17% gewijzigd door Oid op 18-06-2011 13:04]

[RobIII]

Oid schreef op zaterdag 18 juni 2011 @ 12:58:
Zo heb ik het nu ook gebouwd.

Waar MueR op doelt is dit:

PHP:

$sortcol = strtolower($_GET['sortbyfield']));
if (!in_array($sortcol, array('col_x','col_y','col_z','col_foo','col_bar'))
  $sort_col = 'col_x';

$query = 'select sumthin, sumtin from whooptiedoo order by ' . $sortcol;
//execute $query

of zoiets:

PHP:

$sortcol = strtolower($_GET['sortbyfield']);

switch ($sortcol) {
    case 'col_y':
    case 'col_z':
    case 'col_foo':
    case 'col_bar':
       break;
    default:
      $sortcol = 'col_x';
}

$query = 'select sumthin, sumtin from whooptiedoo order by ' . $sortcol;
//execute $query

En dat kan (en is ook veilig), maar is in sommige gevallen wat meer werk wanneer je ergens een kolommetje bij knutselt. Je moet dan niet vergeten deze, naast 'm natuurlijk in je SELECT op te nemen, ook aan de switch (of array) toe te voegen; anders kun je er nog steeds niet op sorteren.

Een andere manier is het ophalen van de beschikbare veldnamen en dan kijken of de userinput daarin voorkomt. Maar dat zul je dan wel een beetje slim moeten (lees: willen) doen, anders doe je voor elke query een extra roundtrip naar de DB. Maar dat kun je prima cachen ofzo.

Of je gebruikt gewoon een fatsoenlijke O/R Mapper; daar zijn die dingen voor (en meer). Maar daar ging 't topic niet over

Oid schreef op zaterdag 18 juni 2011 @ 12:58:
Waarom via user_input:

Euh, dat was jouw aanleiding om een topic te openen?

Oid schreef op zaterdag 18 juni 2011 @ 12:58:
http://datatables.net/rel...ver_side/server_side.html gebruik ik voor de overzichten van data, de sortering is gebasseerd op $_GET

En $_GET is geen user input? En waar is PDO opeens gebleven?
Je laat serieus PDO vallen hiervoor? Omdat je het niet voor elkaar krijgt om een kolom te sorteren? Daar zou voor mij toch echt heel wat meer voor nodig zijn dan zoiets simpels (dat, zoals je in dit topic hebt gezien, prima op te lossen is of omheen te werken). Nee, sorry, ik had hier meer van verwacht/gehoopt...

Daarbij bevat die code nog een aantal behoorlijke WTF's ook..

PHP:

        $sLimit = "LIMIT ".mysql_real_escape_string( $_GET['iDisplayStart'] ).", ".
            mysql_real_escape_string( $_GET['iDisplayLength'] );

"Als we overal maar mysql_real_escape_string omheen flikkeren zitten we altijd safe" Dat ik een complete dump kan maken van 1.000.000.000 records door even iDisplayLength te forceren is ook niet belangrijk. En zo zie ik er nog wel een hoop.

[Voor 58% gewijzigd door RobIII op 18-06-2011 14:09]

[Voutloos]

Precies, mysql_real_escape_string is voor het écht escapen van strings. Als dat niet als string in je query staat (quotes eromheen dus) ben je alsnog gevoelig voor SQL injection.
Naast grote waardes zijn wil je ook negatieve waardes vermijden, want die pikt mysql niet.

En last but not least moet je enkel (mysql) errors tonen tijdens het devven, en niet zoals het hier op deze publieke server staat. Dev dan op een andere locatie, of doe desnoods iets op basis van je eigen ip.

[Oid]

RobIII schreef op zaterdag 18 juni 2011 @ 13:47:
[...]

Waar MueR op doelt is dit:

PHP:

$sortcol = strtolower($_GET['sortbyfield']));
if (!in_array($sortcol, array('col_x','col_y','col_z','col_foo','col_bar'))
  $sort_col = 'col_x';

$query = 'select sumthin, sumtin from whooptiedoo order by ' . $sortcol;
//execute $query

of zoiets:

PHP:

$sortcol = strtolower($_GET['sortbyfield']);

switch ($sortcol) {
    case 'col_y':
    case 'col_z':
    case 'col_foo':
    case 'col_bar':
       break;
    default:
      $sortcol = 'col_x';
}

$query = 'select sumthin, sumtin from whooptiedoo order by ' . $sortcol;
//execute $query

En dat kan (en is ook veilig), maar is in sommige gevallen wat meer werk wanneer je ergens een kolommetje bij knutselt. Je moet dan niet vergeten deze, naast 'm natuurlijk in je SELECT op te nemen, ook aan de switch (of array) toe te voegen; anders kun je er nog steeds niet op sorteren.

Een andere manier is het ophalen van de beschikbare veldnamen en dan kijken of de userinput daarin voorkomt. Maar dat zul je dan wel een beetje slim moeten (lees: willen) doen, anders doe je voor elke query een extra roundtrip naar de DB. Maar dat kun je prima cachen ofzo.

Dat heb ik ook gedaan.

[...]

Euh, dat was jouw aanleiding om een topic te openen?


[...]

En $_GET is geen user input? En waar is PDO opeens gebleven?
Je laat serieus PDO vallen hiervoor? Omdat je het niet voor elkaar krijgt om een kolom te sorteren? Daar zou voor mij toch echt heel wat meer voor nodig zijn dan zoiets simpels (dat, zoals je in dit topic hebt gezien, prima op te lossen is of omheen te werken). Nee, sorry, ik had hier meer van verwacht/gehoopt...

Ik denk dat je mij verkeerd begrijpt, ik blijf PDO gebruiken, dit is voorbeeldcode van datatables wat ik gebruik, uiteraard heb ik de code zelf helemaal aangepast/beveiligd met checks van input e.d.

Daarbij bevat die code nog een aantal behoorlijke WTF's ook..

PHP:

        $sLimit = "LIMIT ".mysql_real_escape_string( $_GET['iDisplayStart'] ).", ".
            mysql_real_escape_string( $_GET['iDisplayLength'] );

"Als we overal maar mysql_real_escape_string omheen flikkeren zitten we altijd safe" Dat ik een complete dump kan maken van 1.000.000.000 records door even iDisplayLength te forceren is ook niet belangrijk. En zo zie ik er nog wel een hoop.

Uiteraard test ik dit en maak ik zelf een check daar omheen.

Voutloos schreef op zaterdag 18 juni 2011 @ 15:40:
Precies, mysql_real_escape_string is voor het écht escapen van strings. Als dat niet als string in je query staat (quotes eromheen dus) ben je alsnog gevoelig voor SQL injection.
Naast grote waardes zijn wil je ook negatieve waardes vermijden, want die pikt mysql niet.

En last but not least moet je enkel (mysql) errors tonen tijdens het devven, en niet zoals het hier op deze publieke server staat. Dev dan op een andere locatie, of doe desnoods iets op basis van je eigen ip.

Ik gebruik PDO, zie TS.

Errors toon ik ook niet op de publieke website, alleen in de dev omgeving.

[Voor 12% gewijzigd door Oid op 18-06-2011 16:28]