Keskustelu: Nettisivujen teko: Varausjärjestelmän rakenne

Rakentelen pientä varausjärjestelmää, jonka avulla etukäteen määritellystä listasta saa valita haluamansa lennon virtuaaisesti. Käyttäjät antavat oman id-numeronsa, sekä email-osoitteen. Vaatimuksena on 6-numeroinen luku id:i, sekä muodollisesti oikea email-osoite.

Varmistaminen toteutetaan seuraavasti:

<?php
if(strlen($_POST['vid']) !== 6 || (!is_numeric($_POST['vid']))) {
	echo "<p>Your id number should contain 6 numbers <a href=\"kellotesti.php\">return</a></p>";
} else if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
	echo "<p>Your email is not correct <a href=\"kellotesti.php\">return</a></p>";
} else {
    /** Kaikki kunnossa varataan ja merkitään asianomaiset tiedot kantaan **/
}
?>

Lennot ovat listattu allekain, sekä jokaisella rivillä on perustiedot ja kaksi tekstikenttää syöttöä varten. Ensimmäinen on nimeltään 'vid' ja toinen 'email'. Kun käyttäjä klikkaa vid-kenttää, niin jquery-kirjaston avulla tehdään tietueen lukitus. Tällöin kukaan ei voi varata samaa lentoa, koska syöttökenttiä ei tulosteta tällöin.

Mikäli käyttäjällä ei ole javascriptiä käytössä, niin lukitusta ei (tietenkään) tehdä, mutta se on sitten käyttäjän oma ongelma, jos joku toinen varaa lennon tälä välin. Lukitus on voimassa klikkauksesta 60 sekuntia.

Osaisitteko näiden tietojen perusteella arvioida mahdollisia sudenkuoppia, joita ei ole tullut otettua huomioon. Tiedossa on sellainen fakta, että käyttäjien antamia tietoja ei ole mahdollista verifioida mitenkään täysin luotettavasti. Käyttäjät saavat antaa siis mitä tahansa (muodollisesti oikeita) tietoja.

Tietueen lukitus näyttää epäilyttävältä. Luulen että lähtökohtaisesti on parempi näyttää esim. joku indikaattori että tätä kyseistä kenttää on mahdollisesti joku muukin varaamassa, mutta näyttää kenttä silti ja sallia varaukset siihen niin kauan kun sen tosiaan on varaamaton. Väkevä lukitus voi altistaa jo tahattomalle palvelun estolle.

Onko käyttäjätunnistus tarkoituksella noin löyhä?

Itse koodista sen verran, että tsekkaa stringit mitä formilta tulee (ne eivät välttämättä ole stringejä). is_numeric ei taida olla sitä mitä haetaan?

Kiitos kommentistasi timoh

timoh kirjoitti:

Tietueen lukitus näyttää epäilyttävältä. Luulen että lähtökohtaisesti on parempi näyttää esim. joku indikaattori että tätä kyseistä kenttää on mahdollisesti joku muukin varaamassa, mutta näyttää kenttä silti ja sallia varaukset siihen niin kauan kun sen tosiaan on varaamaton. Väkevä lukitus voi altistaa jo tahattomalle palvelun estolle.

Tätä mietin itsekin. Onko tällä tavalla mahdollista, että ensimmäisenä kenttää klikannut käyttäjä ei ehdikään varaamaan haluaamansa lentoa? Kenttä kuitenkin vapautuu käyttöön joka tapauksessa minuutissa, jos käyttäjä haluakaan sitä.

timoh kirjoitti:

Onko käyttäjätunnistus tarkoituksella noin löyhä?

On tarkoituksella siinä mielessä, että ei ole tiedossa mitään varmaa tapaa tunnistaa käyttäjää. Varaajapooli tosin on verraten aika pieni, joten väärinkäytöksiä ei luultavasti ilmene.

timoh kirjoitti:

Itse koodista sen verran, että tsekkaa stringit mitä formilta tulee (ne eivät välttämättä ole stringejä). is_numeric ei taida olla sitä mitä haetaan?

Eikös tuo is_numeric tarkista syötteen olevan vain numeroita? edellä oleva pituuden tarkistus varmistaa, että syöte on juuri 6-merkkinen. Onko olemassa jokin syöte, joka näillä varmistuksilla tunnistuisi väärin?

Harrastuskäyttöön ihan hyvä. Yrityksen toimintaan en tosin tuota kelpuuttaisi.

Teuro kirjoitti:

Eikös tuo is_numeric tarkista syötteen olevan vain numeroita? edellä oleva pituuden tarkistus varmistaa, että syöte on juuri 6-merkkinen. Onko olemassa jokin syöte, joka näillä varmistuksilla tunnistuisi väärin?

Kun kyseessä on käyttäjän id-numero niin desimaalit eivät kai ole sallittuja?
1234.5 esimerkiksi menee läpi.

Teuro kirjoitti:

Tätä mietin itsekin. Onko tällä tavalla mahdollista, että ensimmäisenä kenttää klikannut käyttäjä ei ehdikään varaamaan haluaamansa lentoa? Kenttä kuitenkin vapautuu käyttöön joka tapauksessa minuutissa, jos käyttäjä haluakaan sitä.

Näin on. Luulen että parempi jos näytät pelkän infon niiden rivien kohdalla mitä ollaan jo muiden toimesta varaamassa. Jos joku kuitenkin haluaa tämän kesken olevan varauksen aikana itse koittaa varata samaa riviä niin OK. Ehtii jos ehtii. Sama sitten toisin päin, että jos alkaa varaamaan riviä, niin on kuitenkin mahdollisuus että joku muu sen ehtii sillä välin varata ("kisa" tuossa on joka tapauksessa, alkoi se sitten pelkällä hiirellä klikkauksella tai näpyttelystä ja submitista).

is_numeric tosiaan hyväksyy kaikenlaisia numeerisia inputteja, eli ei rajoitu pelkkiin numeromerkkeihin.

Hyviä huomioita asian tiimoilta. Riittäisikö sitten jos tulevan luvun ajaisi intval() funktion läpi, josta mahdolliset desimaaliosat jäävät pois? Tällöin syöte olisi ainakin muodollisesti oikein.

Quick&dirtynä ctype_digit((string) $_POST['vid']). Olettaen että esim. '000001' kelpaa. Sitten tuon jälkeen vielä se strlen.

intval-funktion jälkeen jäisi ainakin vielä negatiiviset arvot kuten -12345

Säännöllisten lausekkeiden opettelu oikeasti kannattaa, niin ei tarvitse kikkailla monella eri funktiolla ja miettiä, mitä ne nyt tarkalleen tekevät tai eivät tee tai pitääkö ehkä tehdä monta erillistä tarkistusta. Tämäkin tarkistus onnistuu näin helposti:

if (!preg_match('/^[0-9]{6}$/s', $_POST["vid"])) die();

Toisaalta minusta on outoa, että mitään id:n muotoa edes tarkistetaan. Eikö olisi kaikin puolin järkevämpää vain katsoa, löytyykö kyseinen id tietokannasta? Vai onko id mielivaltainen ja käyttäjän vapaasti valittavissa?

Teuro kirjoitti:

Hyviä huomioita asian tiimoilta. Riittäisikö sitten jos tulevan luvun ajaisi intval() funktion läpi, josta mahdolliset desimaaliosat jäävät pois? Tällöin syöte olisi ainakin muodollisesti oikein.

PHP:n tapaan parsia lukuja ei voi luottaa. Ei-numeeriset merkkijonot voivat castautua kokonaisluvuiksi tai olla castautumatta (ts. tulos on 0).

foreach (array('2', 'a', 'a2', '2a', "\n  2a") as $s) {
    print intval($s) . PHP_EOL;
}

Tulos:
2
0
0
2
2