PHP and the dollar sign

Sunt sigur ca orice junior stie de ce trebuie sa valideze, filtreze si escapeze inputul. Nu se poate sa nu fi auzit de la cei mai batrani povesti de groaza cu balauri, legate de sql injection si xss attacks.

Dar asta nu inseamna neaparat ca si stiu ce anume sa sanitizeze. Deasemenea, din ce in ce mai multi programatori invata de pe internet tot ce stiu. Iar nu toti au norocul sa lucreze cu persoane care sa spuna astfel de povesti de groaza.

Assumtions are the cause of most problems.

Daca nu stiu, sa invete, ca de aia au creier si e nevoie de ei. Altfel, daca softurile erau asa destepte, programau chiar ele, nu mai era nevoie de programatori umani.

Limbajul te poate ajuta pana la un punct, daca trece de punctul ala incepe sa te incurce. Pana la urma limbajele astea sunt create ca unelte pentru profesionisti. Daca nu esti in stare sa intelegi cum functioneaza si e prea complicat pentru tine, poti sa ramai la VisualBasic. Din 4 click-uri e gata “programul” :slight_smile:

Nu asta e ideea, deoarece nu au cum sa invete daca nu stiu de respectivele probleme. Iar, de obicei, in documentatie si tutorialele de pe net nu gasesti explicatii detaliate. Plus ca, in general, documentatia si tutorialele necesita contact interuman minim, ceea ce face ca multe persoane sa caute sa interactioneze cu alti programatori fie offline, fie online (exemplu fiind forumul acesta).

Dar deja am dat in offtopic.

Ideea este sa oferim exemplele detaliate, sa punem intrebari la alor caror raspuns sa se gandeasca, si sa ii invatam, direct sau indirect, sa isi puna intrebari de genul si pentru alte situatii. Iar asa ajungem la overengineering. (Si asta este o problema.) Practic, cum ai da-o, probleme tot sunt.

Ne invartim in cerc, eu dau un exemplu tu aduci altul + argumentum ad ignorantiam (ca poti face aceasta greseala doar daca iti lipseste half a brain or something) facand complet abstractie ca am intalnit destul de des aceasta problema (anectodal evidence, sure you can ignore this) dar chiar exista o functie implementata intr-o librarie pt. ea (actual evidence based on empirical data).

si in alte limbaje (Java, Python) similar thing:

But I strongly recommend to use a whitelist of classnames.

via java - Creating an instance using the class name and calling constructor - Stack Overflow

again cineva mentioneza macar the whitelist part in loc de “wtf!!! mind blown”

via Creating PHP class instance with a string - Stack Overflow

Nu ne invartim in niciun cerc. Ai dat mai sus un exemplu. Te-am intrebat de cateva ori care e vulnerabilitatea acolo. Guess what, n-am primit niciun raspuns. Hai sa dezbatem concret, care e problema cu $instance = new $class_name. In afara de poveştile cu baubau, cu intrarile nevalidate. Ca daca nu validezi intrarea, ruby iti crapa cu fix aceeasi eleganţă.

That is why it’s a bad practice! AAAAA :)))) Daca uiti sa aplici o validare sau sa faci whitelist este un vector de atac sau at best weird behaviours.

Am dat peste n de astfel de oameni iar intr-o librarie dintr-un framework exista o functie pt. asta what more evidence do you need?

Stai putin sa inteleg. Deci daca in ruby uiti sa faci o validare nu se intampla nimic rau? O sa curga in continuare lapte si miere? Sau cum? A nu valida inputul nu e “bad practice”?

A folosi combinatia de let's turn a string into a constant + whitelisting (oricum i-ai zice) pentru cateva cazuri in loc de un banal switch sau case este bad practice.

Problema este ca faci o validare pt. whitelisting al unui pattern care poate fi usor inlocuit cu un switch sau case - diferenta e ca in cazul unui switch sau case nu ai cum sa uiti sa faci validarea.

Daca ai cateva variante, foloseste un construct simplu al limbajului nu metaprogramming.

Daca iti faci propriul DSL sau framework then by all means, go nuts.

2 Likes

Tot repeti ca e bad practice, si tot nu-mi spui de ce. Ruby n-o ia razna daca nu validezi?

1 Like

Pentru ca folosesti wrong tool for the job in caz ca nu era clar ce am zis mai sus:

  • folosesti un construct care tine de metapogramming si are propriile nuante care incepatorii sau cum vrei sa le zici uita de ele (adica uita sa si valideze input-ul)
  • motivul pt. care este folosit este: wow factor si writing clever code for the sake of that

S-au incalzit spiritele pe thread-ul asta, si e pacat, ca a inceput foarte frumos.

Am sa incerc eu sa raspund.

In general, generarea de cod dinamic, e un anti-pattern. Ma refer aici la chestii precum eval, constructie de sql de mana, invocare de functii pe baza unei valori dintr-o variabila[1], invocarea unei metode dintr-o variabila, generare de cod dinamic, etc.

Motivul principal este ca este extrem de usor sa te impusti in picior. Si asta ca expert nu ca novice.

O prima problema este securitatea. Daca invocarea dinamica este bazata pe input de la utilizator, trebuie sa ai extrem de mare grija ce faci. Exemplele date pe thread sunt simple, si oricine vede daca ceva e OK sau nu. Intr-un codebase de 1 milion de linii la care lucreaza 20 de oameni treaba nu mai e asa. Chiar si ceva simplu ca “toate controller-ele” din proiect s-ar putea sa fie greu de obtinut. Cineva o sa uite sa adauge ceva in white-list, altcineva o sa uite sa stearga ceva din whitelist, productivitatea scade si attack surface-ul creste. Plus, cand set-ul de validare o sa aiba 1000 sau mai multe elemente, cuiva o sa-i vina idea buna sa puna si expresii regulate acolo. Alt canal de atac. Cand ai o alternativa simpla ca un map de la rute la handlere (cum ai in node/express, ASP.NET MVC), sau anotari (ASP.NET MVC) zic ca nu merita complexitatea.

O alta problema este performanta. Mai toate limbajele folosite pe partea de web sunt interpretate cu un compilator de bytecode JIT mai mult sau mai putin bun. Dar chiar si cele mai slabe apreciaza cand ai un apel la o functie cunoscuta la compile-time (constructorul controllerului), in loc de un eval/lookup in scope urmata de apelul unei functii date de rezultat. Cand treaba asta se intampla in hot-path - pentru fiecare request pe care-l procesezi, chiar conteaza.

De asemenea, in cazul unui switch sau unui map, multe editoare o sa-ti arate ca un controller / functie etc. este folosit, de cate ori este etc. Cand treaba e dinamica (sau cu anotari, din nefericire), pierzi informatia asta. Si ai un set de functii/clase care sunt folosite, dar pe care uneltele de analiza statica nu le pot identifica ca atare.

Sigur, nu inseamna ca sunt lucruri de evitat complet. Au si ele rostul lor, altfel n-ar fi in limbaj. Cateodata chiar trebuie sa evaluezi un program dat de utilizator (pe HackerRank sau jsfiddle de exemplu). Cateodata trebuie sa generezi cod automat, pentru ca profilul de performanta nu poate fi atins altfel. Dar de cele mai multe ori, exista o alternativa mai simpla, mai performanta si mai buna.


[1] ceva de genul

x = ‘sqrt’
$x(100)
Asemanator cu invocarea de constructor din variabila.

5 Likes

De acord cu tot ce ai zis. Eu doar incerc sa spun ca feature-ul nu este rău per se. Toată şmecheria este sa-l folosesti cu cap. Ca asa o gramada de tehnologii sau mecanisme ar putea fi considerate evil. Multithreading-ul e evil, metaprogramming-ul e evil, sql-ul e evil, polimorfismul e evil, overload-ul operatorilor e evil, functiile lambda sunt evil… Ce ne mai ramane? :slight_smile:

pai mare parte din ele (indeosebi ultimele 3) chiar nu se recomanda a fi folosite in mod uzual de programatorii care dezvolta o amarata aplicatie web

1 Like

sunt utile in middle steps of refactoring a huge class/method.sau cand nu ai destule date inca unde ar trebui sa existe o metoda.

1 Like

Neah, functiile lambda chiar imi plac. Mai ales la js, cand poti sa atasezi functiile lambda direct la evenimentele asincrone. Mi se pare mult mai elegant si mai concis sa scrii ca mai jos, decat sa ai functie separata ca handler. Mult mai rau era daca aveam multithreading in aplicatiile web, sa vezi atunci cum vânam bugurile pe 7 fire de executie care sa bumbăcesc intre ele :slight_smile:

$.post("/url", { arg: val }, function(response)
{
   if(response.error)
        alert("Eroare: " + response.error);
    else
        alert("Raspuns: " + response.data)
}, "json");

corect, cu mentiunea ca rareori pasul final (commitul rebased care ascunde intermediate steps) le mai contine

eu in decursul timpului am ajuns sa vad orice magic dintr-asta (ma refer in general la ce discutati pe thread) ca un code smell; cu cat mai mult magic, cu atat mai greu de onboard cineva nou si de mentinut

e f usor sa aratam fiecare cat de buni suntem in a implementa rapid ceva specific, insa care este costul rapiditatii dpdv al mentinerii pe termen lung al acelei bucati de cod? in functie de experienta fiecaruia (masurata in complexitatea problemelor intalnite, nu in ani) codul respectiv va arata diferit; am observat ca, in timp, seniorii invata sa numeasca variabilele mai eficient, sa foloseasca cele mai simple constructii ale limbajului, etc

5 Likes

Depinde si cat de disciplinat e cel care scrie codul. Eu nu mai lucrez in echipa de peste 10 ani (sunt lup singuratic), dar chiar si asa, daca folosesc chestii mai exotice sau constructii mai putin intuitive le documentez foarte atent, ca sa nu stau doua ore sa descifrez ce naiba am facut acolo, peste 2 ani.

Dap, de obicei blochez un PR daca:

  • non-idiomatic code
  • overly complex i.e. is over-engineered sau under.
  • sub-optimal naming
  • too clever just for the sake of it
  • cannot be understood easily
    etc.

Unele proiecte nu permit asta insa acolo merg pe ideea de incremental refactoring (care evident merge si sus insa nu este atat de pronuntat)

Codul scris cat mai simplu si close to the problem’s domain should almost be self-documenting if not the tests should fully clear it up.

Ca regula generala, da. Codul este auto-explicativ. Dar mai sunt cazuri cand codul e incalcit. Fie esti constrans de performanta de executie, fie algoritmul e complicat etc.