Proszę o ocenę kodu pod względem optymalności oraz bezpieczeństwa. Parser miał być prosty i bezpieczny. Udało się cały kod zmieścić w 66 linijkach także dużo sprawdzania nie będzie ;-)
Tutaj znajduje się cały kod wraz z opisem:
https://github.com/ToTamir/BBCode-Parser
Pozwalanie na wrzucenie do bbcode do obrazka dowolnego url usuwa z tej klasy tag "bezpieczna"
Brak klas na elementach ktore generujesz uniemozliwia mi jakiekolwiek ostylowanie tego bbcode
No i testowales w ogole to:
$patterns[] = '/\r\n/';
? Odnosze wrazenie ze nie powinno zadzialac. No i co z sytuacja gdy ktos bedzie mial poprostu tylko znacznik \n ?
No i na koniec bbcode to juz raczej wypada z obiegu. Teraz bardziej przyjaznym dla uzytkownika jest Markdown
Na co należałoby twoim zdaniem zwrócić uwagę przy filtrowaniu url dla obrazów?
To przejście do nowej lini działa poprawnie, a przypadek z samym znacznikiem \n uwzględnie w najbliższym czasie.
I jeszcze sam kod:
niby piszesz pod php7 i niby funkcja ma zwracac tylko string, ale dajesz
return preg_replace()
Jakbys zajrzal do manuala to bys zobaczyl ze preg_replace moze tez zwrocic cos innego niz string a tym samym narazasz mnie, jako uzytkownika twojej aplikacji, na FATAL ERROR na stronie.
nie array()
a []
krotka notacja juz od dawna jest standardem
no i nie
$ar[] = ....
$ar[] = ...
a:
$ar = [
...
...
]
ps: podobnie z preg_replace_callback ktore moze zwrocic nie string. Warto dbac o takie szczegoly w szczegolnosci ze okreslasz swoja funkcje mianem "bezpiecznej"
Poprawiłem tablice oraz uwzględniłem same \n .Wygląda to obecnie tak:
function ParseBBCode(string $string): string { $string = http://www.php.net/htmlspecialchars($string, ENT_QUOTES, 'UTF-8', true); $patterns = [ '/\[ol\][\s\S]+?\[\/ol\]/', '/\[ul\][\s\S]+?\[\/ul\]/', '/\[table\][\s\S]+?\[\/table\]/' ]; $string = http://www.php.net/preg_replace_callback($patterns, function($matches){return http://www.php.net/preg_replace('/\s+/', ' ',$matches[0]);}, $string); $patterns = [ '/\[b\](.*?)\[\/b\]/', '/\[i\](.*?)\[\/i\]/', '/\[u\](.*?)\[\/u\]/', '/\[s\](.*?)\[\/s\]/', '/\[j\](.*?)\[\/j\]/', '/\[size=([\d.]+)\](.*?)\[\/size\]/', '/\[color=(([a-z]+)|(#[0-f]{6}?)|(rgb\(\d{1,3}?\,\d{1,3}?\,\d{1,3}?\)))\](.*?)\[\/color\]/', '/\[center\](.*?)\[\/center\]/', '/\[left\](.*?)\[\/left\]/', '/\[right\](.*?)\[\/right\]/', '/\[quote=(.*?)\](.*?)\[\/quote\]/', '/\[quote\](.*?)\[\/quote\]/', '/\[url=(.*?)\](.*?)\[\/url\]/', '/\[img=(.*?) alt=(.*?)\]/', '/\[youtube=(.*?)\]/', '/\[li\](.*?)\[\/li\]/', '/\[td\](.*?)\[\/td\]/', '/\[ol\](.*?)\[\/ol\]/', '/\[ul\](.*?)\[\/ul\]/', '/\[table\](.*?)\[\/table\]/', '/\[tr\](.*?)\[\/tr\]/', '/\[th\](.*?)\[\/th\]/', '/\[code\]/', '/\[\/code\]/', '/\r\n/', '/\n/' ]; $replacements = [ '<strong>$1</strong>', '<em>$1</em>', '<span style="text-decoration:underline;">$1</span>', '<del>$1</del>', '<span style="text-align:justify;text-justify:inter-word;">$1</span>', '<span style="font-size:$1rem;">$2</span>', '<span style="color:$1;">$5</span>', '<div style="text-align:center;">$1</div>', '<div style="text-align:left;">$1</div>', '<div style="text-align:right;">$1</div>', '<blockquote><strong>$1:</strong><br>$2</blockquote>', '<blockquote>$1</blockquote>', '<a href="$1">$2</a>', '<img src="$1" alt="$2">', '<iframe src="https://www.youtube-nocookie.com/embed/$1" allow="autoplay; encrypted-media" allowfullscreen></iframe>', '<li>$1</li>', '<td>$1</td>', '<ol>$1</ol>', '<ul>$1</ul>', '<table>$1</table>', '<tr>$1</tr>', '<th>$1</th>', '<code>', '</code>', '<br>', '<br>' ]; return http://www.php.net/preg_replace($patterns, $replacements, $string); }
Ale naprawde taki problem zamiast
return preg_replace($patterns, $replacements, $string);
dac
return (string)preg_replace($patterns, $replacements, $string);
I juz kod jest zabezpieczony. Tobie sie teraz wydaje ze ten NULL nie wystapi nigdy, moze i slusznie, ale nie jestes w stanie przewidziec co sie stanie u kogos za rok. Ja po "paru" latach programowaniu wiem ze po jakims czasie w skrypcie potrafia sie pojawic nieoczekiwane bledy. Naprawde wiec jest takim duzym narzutem danie rzutowanie na string, tak na wszelki wypadek? No nie jest. Skoro funkcja ma zwracac string to niech zwraca string na 100% zawsze.
Co do obrazow to co ja ci poradze... No sprawdzaj czy to obraz czy nie Moze przez to bedzie dzialac to dluzej ale na pewno bedzie bezpieczniej.
Od biedy mozesz patrzec na rozszerzenie ale w dzisiejszych czasach to juz dawno nie jest wyznacznikiem.
Siemanko! Czemu nie ma testów jednostkowych?
Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)