Autor Zpráva
nadsenec
Profil *
Ahoj,

snažím se z kódu, který se mi často opakuje (sanitace dat z formuláře) udělat fci. Kód, pokud není ve fci, funguje a vypadá takto...

if (!empty($_POST['formName'])) {    
    $formName = filter_var($_POST['formName'],FILTER_SANITIZE_STRING);
    $formName = trim($formName);
    echo $formName; //pouze pro testování
} else { 
    $formName = 'prazdne';
    echo $formName; //pouze pro testování
}

...ovšem pokud z něj udělám fci...

// Funkce k sanitaci řetězce
function sanitizeString ($stringToSanitize) {
    
if (!empty($_POST['stringToSanitize'])) {    
    $stringToSanitize = filter_var($_POST['stringToSanitize'],FILTER_SANITIZE_STRING);
    $stringToSanitize = trim($stringToSanitize);
    echo $stringToSanitize."<br>"; //pouze pro testování
} else { 
    $stringToSanitize = 'prazdne';  
    echo $stringToSanitize."<br>"; //pouze pro testování
}

} // konec fce sanitizeString

...a pak ji zavolám...

sanitizeString ("formName");

...tak se mi řetězec z POSTu pořád tváří jako prázdný (fce vypíše "prazdne").

Nevíte, prosím, někdo proč tomu tak je? :-) Díky.
Dan Charousek
Profil
Protože nikde tu hodnotu nevracíš.
Vlož return $stringToSanitize; na konec té funkce.
xROAL
Profil
Pretože if (!empty($_POST['stringToSanitize'])) { kontroluje $_POST['stringToSanitize'] bez ohľadu na to, aký argument do funkcie dáš. Ak chceš, aby sa ti tam dosadilo "formName", tak to samozrejme musí byť if (!empty($_POST[$stringToSanitize])) { .

Edit: to isté na riadku 5, rovnaká chyba.
nadsenec
Profil *
Jáááááááááááááááááááj, no jasně, xROAL na to kápnul. To je ta má slepota. :-) Dík moc!


Jo a Dan Charousek taky. Pokud tam odstraním to echo pro testovací účely, musím vracet hodnotu. Čili by to asi mělo být nějak takto...

// Funkce k sanitaci řetězce
function sanitizeString ($stringToSanitize) {
    
if (!empty($_POST[$stringToSanitize])) {    
    $stringToSanitize = filter_var($_POST[$stringToSanitize],FILTER_SANITIZE_STRING);
    $stringToSanitize = trim($stringToSanitize);
} else { 
    $stringToSanitize = 'prazdne';  
}

return $stringToSanitize;
} // konec fce sanitizeString

Oběma moc děkuji!
mimochodec
Profil
nadsenec:
To není moc pěkné, mít v jedné proměnné dvě tak snadno omylem zaměnitelné věci.
Keeehi
Profil
nadsenec:
Mimo jiné by tam nemělo být ani to "prázdné", jelikož to tam logicky nepatří. Sanitize funkce by měla jen měnit daný string. Ne vytvářet nový. Ani ta závislost na globální proměnné není dobře. Takže nakonec z toho zbyde:
function sanitize($stringToSanitize) {
    return trim(filter_var($stringToSanitize,FILTER_SANITIZE_STRING));
}
A to všechno ostatní patří jinam. Kam? To záleží na ostatním kódu.
nadsenec
Profil *
To "prázdné" jsem tam měl jen pro testování a tak moc rozepsané to mám, abych to nejdříve pochopil. Teprve když tomu budu rozumět, můžu to zkracovat. :-) Celý ten kód (fce) je ve skriptu, který se stará o zpracování a ta závislost na globální proměnné tam je jen proto, aby se to vykonávalo pouze ve chvíli, kdy je POST vyplněný. On sice v tomto případě bude vyplněný vždy, protože čistím pole, které je ve formuláři jako povinné, ale všude čtu, že by se ta podmínka měla psát i tak.

Rád si ale nechám vysvětlit nějaký jiný, lepší způsob. Díky!
mimochodec
Profil
nadsenec:
Prostě bych to napsal tak, abys mohl používat sanitizeString ($_POST['xxx']).
Alphard
Profil
Začíná tady vznikat zmatek, každý radí něco jiného a z původního tazatelova kódu se ztratila část funkčnosti.

[#1] dělá dvě věci.
1. Ověřuje prázdnost (neexistenci) daného klíče v superglobálním poli $_POST a dosazuje defaultní hodnotu.
2. Nějakým definovaným způsobem ošetřuje určité znaky.

Naimplementováno by to mělo být ve dvou funkcích. Řekněme getPost($key, $default = null) a sanitize(), viz [#6] Keeehi.

Potom by se volalo
$osetrenyString = sanizite(getPost('formName', 'prazdne'));

Doplňuji návrh na další budoucí úpravu. Ošetřování řetězců se vždy dělá jinak pro každý kontext (html, js, sql, ...), proto by bylo vhodné do názvu funkce sanitize přidat informaci o tom, co konkrétně dělá.
nadsenec
Profil *
Alphard: Ano, kód by měl ověřit, zda je POST prázdný, pokud není, tak očistit (a poté zapsat do db), pokud je, tak stanovit defaultní hodnotu. Pokud to chápu správně, tak by mělo být dobrým zvykem kód rozdělit do dvou fcí a to i přesto, že jde o tak jednoduchou záležitost. Čistě proto, abych se naučil správně k zápisu fcí přistupovat? Jinými slovy - každá fce by měla vykonávat pouze jednu věc? Díky.
Alphard
Profil
Z obecného pohledu:
Ano, každá funkce/třída by měla mít jednu zodpovědnost (Single responsibility principle, ačkoliv to bývá formulováno pro objektové programování, lze to rozumně aplikovat i na jednoduché funkce).
Pozor, tím neříkám, že by se všude mělo řetězit 10 funkcí, určitě ne. Jen se více jednoduchých funkcí spojí do jedné komplexnější, která bude opět dělat jednu věc a podle ní bude pojmenovaná.
Ve vašem případě má každá z mnou popsaných [#9] funkčností smysl sama o sobě, proto jsou oddělené do funkcí. Kdyby neoddělitelně patřily k sobě, lze zvažovat jejich ponechání v jedné funkci.

Konkrétně k vašemu případu:
Obecná funkce pro ošetření vstupu do databáze nemůže existovat, escapování musí být vždy kontextově citlivé, takže tam chybí minimálně escapování konkrétní databázovou knihovnou (a ošetření pro kontext html by mohlo být až na výstupu), viz phpfashion.com/escapovani-definitivni-prirucka. Takže
ulozeniDoDatabaze(escapeForSQL(getPost('input')));
echo sanitizeForHTML(nacteniZDatabaze());

Vaše odpověď

Mohlo by se hodit


Prosím používejte diakritiku a interpunkci.

Ochrana proti spamu. Napište prosím číslo dvě-sta čtyřicet-sedm:

0