[PHP] Performance van bestanden

[RickvanHaas]

Leden van mijn website kunnen foto's uploaden die niet voor iedereen zichtbaar zijn. Bij het uploaden wordt er een nieuw record in de tabel 'pics' aangemaakt en wordt het bestand in de root gezet (folder: "verborgen_map"), dus onbereikbaar voor mensen zonder toegang tot de webserver.

Bij het bekijken van de foto wordt een check uitgevoerd of men ingelogd is, en zo ja, wordt de afbeelding getoond. Alle afbeeldingen worden aangeroepen via de file "generate_image.php", waarin de inloggegevens gechecked worden, en de foto getoond wordt. Dit script ziet er (gesimplificeerd, dus alle foutdetectie eruit), zo uit:

PHP:

<?php
//fotogegevens uit database ophalen
$sql_pic = "SELECT id, extension, filesize FROM tabel WHERE id = '".$_GET['id']."'";
if($var_pic = mysql_query($sql_pic))
{
    while($pic = mysql_fetch_object($var_pic))
    {
        //locatie van bestand definieeren
        $pic_location = "verborgen_map/".$pic->id.".".$pic->extension."";
        
        if(file_exists($pic_location))
        {
        
            $fp = fopen($pic_location, "rb"); 
            //header bepalen op basis van extensie
            if($pic_info['extension'] == "gif")
            {
                header("Content-Type: image/gif"); 
            }
            elseif($pic_info['extension'] == "png")
            {
                header("Content-Type: image/png"); 
            }
            else
            {
                header("Content-Type: image/jpeg"); 
            }
            
            header("Content-Length: ".$pic->filesize.""); 
            header("Content-Transfer-Encoding: binary\n"); 
            fpassthru($fp); 
            exit; 
        }
    }
}
else
{
    echo mysql_error();
}

?>

Functioneel gezien werkt het prima, maar het laden van de foto's kan erg lang duren. Hoe zou ik bovenstaand script kunnen aanpassen zodat de boel wat beter performt?

[TeeDee]

Performance kan overal aan liggen. Trage SQL, trage Disk IO, slechte verbinding. Wat is 'kan erg lang duren'? Een paar uur of een paar miliseconden.

[Tharulerz]

Definieer lang duren. Zet eens wat time checks tussen de verschillende stappen. Gaat het fout op het niveau van de query? Op de fopen? etc.

[Manuel]

Zoals Voutloos altijd zegt: 'Meten is weten'

[TeeDee]

Manuel schreef op zondag 07 maart 2010 @ 15:19:
Zoals Voutloos altijd zegt: 'Meten is weten'

Moet je wel weten wat je wil gaan meten...

[Gomez12]

Ik gok zomaar even dat die check om te zien of mensen ingelogd zijn loopt via sessies...

En dat je geen speciale voorzieningen hebt qua sessies waardoor je race-condities / locking krijgt omdat je browser in 1x 8 plaatjes opvraagt. 7 connecties wachten dan totdat het 1e plaatje klaar is...

[djexplo]

Waarschijnlijk is het handig om een "rewrite rule" in je webserver aan te maken die het adres www.website.com/plaatje.jpg omzet in www.website.com/generate_image.php?id=\"plaatje"

dan zal de browser het plaatje gewoon cachen en niet elke keer op vragen ...

[Voutloos]

Manuel schreef op zondag 07 maart 2010 @ 15:19:
Zoals Voutloos altijd zegt: 'Meten is weten'

Ik wou het net zeggen.

TeeDee schreef op zondag 07 maart 2010 @ 15:35:
[...]

Moet je wel weten wat je wil gaan meten...

Dan meet je toch een paar dingen. Beetje klikken in firebug, timertje over gehele script, timertje in je db class hatsikidee.

djexplo schreef op zondag 07 maart 2010 @ 16:01:
Waarschijnlijk is het handig om een "rewrite rule" in je webserver aan te maken die het adres www.website.com/plaatje.jpg omzet in www.website.com/generate_image.php?id=\\\\"plaatje"

dan zal de browser het plaatje gewoon cachen en niet elke keer op vragen ...

Het opnieuw opvragen heeft 0.0 met de url te maken. Om te zorgen dat de client kan cachen, moet je als server in de http response headers vertellen dat je kan cachen.
_{Voorgaande zin bevat alle zoektermen}

Wat ik trouwens ook altijd zeg: Sql injection, sql injection, sql injection. Er wordt hier letterlijk user input in de query gestopt. Gebruik in dit geval mysql_real_escape_string() en lees je aub in over het onderwerp, het hoe en waarom. Of lees je in in PDO.

[ Voor 11% gewijzigd door Voutloos op 07-03-2010 19:36 ]

[Manuel]

Hehe Voutloos, zoals je ziet onthoudt zelfs ik jouw posts

Verder kan ik je zeggen, gebruik geen MySQL meer maar eerder Zend_DB / PHP PDO of MySQLi (improved). Dit omdat je daarmee ook prepared statements kunt doen. Hoef je zelf nooit meer te klooien met escape dit en dat.

Nog een puntje. Gebruik mysql_fetch_assoc in plaats van mysql_fetch_object, ook vanwege performance.

[Voutloos]

Over je performance puntje: Hoewel er voor de lol een while in de code staat, gaat het maar om een enkele succesvolle mysql_fetch_* call, en dan is de discussie daarover dus niet relevant, want micro optimalisatie.

[Manuel]

@Voutloos: Alle kleine beetjes helpen zeggen ze vaak.

@OT:
Maar verder lijkt mij dit een kwestie van debuggen waar nou de echte bottleneck zit. Of die nou in de fopen zit of in de MySQL query, we kunnen hier niets mee. (Ik neem aan dat ik voor iedereen spreek als ik zeg dat we met de gegeven info nog steeds niets kunnen beginnen).

[Gomez12]

Tja, mijn glazen bol zegt toch echt dat het hem nergens in de geposte code zit... ( of het is een echt extreme baggerserver met baggerdbase etc erachter, dit kan ( in principe ) geen vertraging opleveren )

[pieturp]

PHP:

$sql_pic = "SELECT id, extension, filesize FROM tabel WHERE id = '".$_GET['id']."'";

En mijn glazen bol zegt me dat je hier wel gevoelig bent voor SQL injection...

[Gomez12]

pieturp schreef op maandag 08 maart 2010 @ 00:40:

PHP:

$sql_pic = "SELECT id, extension, filesize FROM tabel WHERE id = '".$_GET['id']."'";

En mijn glazen bol zegt me dat je hier wel gevoelig bent voor SQL injection...

Tja, sommige lezen ook wel eens een TS :

Dit script ziet er (gesimplificeerd

Maar al zou dit het originele script zijn, leg mij eens uit hoe gevoelig zijn voor SQL-injections je performance omlaag gooit? Veelal komt het je performance juist ten goede omdat veel script-kiddies niet verder komen dan enkel wat delete statements...

offtopic:
Ik begin zo onderhand echt moe te worden van al dat geschreeuw over SQL-injections in voorbeeld code. Ik geloof dat ik volgende keer als ik een stukje code zou post ik er beter gelijk mijn framework van 80k bij kan plakken, dan kan iedereen nagaan of er wel of geen SQL-injections zijn. Ook al hebben SQL-injections zonder verdere aanwijzing niets met de vraagstelling te maken.

Waarschuwen voor SQL-injections is leuk en goedbedoeld, maar de laatste tijd gaat het imho veels te ver.
Je wordt geacht verkorte voorbeeld code te geven, niet live-code die 80k is. Dan kan het best makkelijk zijn om in je voorbeeldcode een SQL-injection te laten zitten gewoon om te laten zien dat je user-input verwerkt...

[GlowMouse]

Gomez12 schreef op maandag 08 maart 2010 @ 00:59:
[...]

Tja, sommige lezen ook wel eens een TS :

[...]

offtopic:
Ik begin zo onderhand echt moe te worden van al dat geschreeuw over SQL-injections in voorbeeld code. Ik geloof dat ik volgende keer als ik een stukje code zou post ik er beter gelijk mijn framework van 80k bij kan plakken, dan kan iedereen nagaan of er wel of geen SQL-injections zijn. Ook al hebben SQL-injections zonder verdere aanwijzing niets met de vraagstelling te maken.

Waarschuwen voor SQL-injections is leuk en goedbedoeld, maar de laatste tijd gaat het imho veels te ver.
Je wordt geacht verkorte voorbeeld code te geven, niet live-code die 80k is. Dan kan het best makkelijk zijn om in je voorbeeldcode een SQL-injection te laten zitten gewoon om te laten zien dat je user-input verwerkt...

Aan de while-loop hier zie je gewoon dat het vrijwel het volledige script is zonder grote vereenvoudigingen.

[Gomez12]

GlowMouse schreef op maandag 08 maart 2010 @ 01:00:
[...]

Aan de while-loop hier zie je gewoon dat het vrijwel het volledige script is zonder grote vereenvoudigingen.

Tja, toch blijf ik erbij dat de fout 99% zeker zit in een stukje wat hieruit gelaten is... Ik zie namelijk geen session_start()

Met Voutloos was de opmerking over SQL-injection gemaakt, hier voor de rest op doorgaan en gelijk maar even een Zend_DB / PHP PDO of MySQLi oid aanraden zonder verdere achtergrond info lijkt me enkel zwaar offtopic

[TeeDee]

Voutloos schreef op zondag 07 maart 2010 @ 19:33:
[...]
Dan meet je toch een paar dingen. Beetje klikken in firebug, timertje over gehele script, timertje in je db class hatsikidee.

Wel erg makkelijk, maar je hebt gelijk. Beetje dit, beetje dat et voila. Punt is: het hoeft niet aan de code te liggen. Een rotte db, trage connectie of trage disk kan er ook voor zorgen natuurlijk.

Nu is het nog wachten op de TS met de testresultaten en meer info.

CptChaos schreef op maandag 08 maart 2010 @ 11:51:
[...]
Punt is, als je niet meet weet je niets en weet je ook niet of je dingen kan vergeten. Meten is namelijk niet alleen weten, maar ook vergeten, omdat je die dingen uitgesloten hebt.

Dat leek mij duidelijk Ander punt dan: zo afgaand alleen op de code zou ik het namelijk niet daar zoeken.

[ Voor 28% gewijzigd door TeeDee op 08-03-2010 14:13 ]

[CH4OS]

TeeDee schreef op maandag 08 maart 2010 @ 11:18:
Wel erg makkelijk, maar je hebt gelijk. Beetje dit, beetje dat et voila. Punt is: het hoeft niet aan de code te liggen. Een rotte db, trage connectie of trage disk kan er ook voor zorgen natuurlijk.

Punt is, als je niet meet weet je niets en weet je ook niet of je dingen kan vergeten. Meten is namelijk niet alleen weten, maar ook vergeten, omdat je die dingen uitgesloten hebt.

[pieturp]

Ik had over 't commentaar van Voutloos heengekeken. Desalniettemin:

Gomez schreef:
Maar al zou dit het originele script zijn, leg mij eens uit hoe gevoelig zijn voor SQL-injections je performance omlaag gooit? Veelal komt het je performance juist ten goede omdat veel script-kiddies niet verder komen dan enkel wat delete statements...

Natuurlijk gaat 't niet om performance, nee. Maar vele tutorials op internet geven echt slechte voorbeelden als 't om security gaat. Je moet mensen daar op wijzen. Er zijn (potentieel) meer mensen die dit topic volgen. Waarschijnlijk zitten daar ook mensen tussen die nog nooit van SQL injection hebben gehoord. Je complete database of site genuked omdat je geen veilige code schrijft is iets wat je kunt voorkomen.

Veelal komt het je performace juist ten goede [...]

Huh Een simpele mysql_real_escape_string() of cast en check op (int) is echt geen performance hogger hoor. Of snap ik nou niet wat je bedoelt?

Tenslotte heeft 't wel of niet hebben van een framework er geen bal mee te maken. Ook zonder kun je prima veilige code schrijven. (Net als dat 't tegenovergestelde ook waar is)

*wacht ook op TS' resultaten

[ Voor 4% gewijzigd door pieturp op 08-03-2010 14:25 ]

[Wolfboy]

pieturp schreef op maandag 08 maart 2010 @ 14:06:
Huh Een simpele mysql_real_escape_string() of cast en check op (int) is echt geen performance hogger hoor. Of snap ik nou niet wat je bedoelt?

Hij bedoelt dat als een paar scriptkiddies wat deletes uitvoeren dat je site dan een heel stuk sneller is

[RickvanHaas]

Dank voor de feedback. De hele while- en SQL-injection disuccsie laat ik even voor wat ie is: ik gaf al aan dat dit de gesimplificeerde versie is (ook alle custom functies die ik gebruik eruit gehaald om de boel duidelijk te houden). Dergelijke security zit in het uiteindelijke script (believe it or not).

Na wat kloten met Firebug lijk ik een verbetering te merken. Firebug gaf aan dat de filesize onbekend was. Nu ik die meegeef in de header, performt ie als een zonnetje. Zou dat het kunnen zijn?

[TeeDee]

RickvanHaas schreef op maandag 08 maart 2010 @ 22:44:
Zou dat het kunnen zijn?

Als dat je probleem oplost, lijkt mij van wel.

Geef je trouwens met

PHP:

header("Content-Length: ".$pic->filesize."");

de filesize al niet mee?

[ Voor 22% gewijzigd door TeeDee op 08-03-2010 23:01 ]

[pieturp]

RickvanHaas schreef op maandag 08 maart 2010 @ 22:44:
Dergelijke security zit in het uiteindelijke script (believe it or not).

Nice

Ook fijn dat de oorzaak toch nog is gevonden