Skrypt konfiguracji ogólnej pisany przez profesionalistów... Opcje

[Adi32]

Sam nie jestem profesjonalistą ale ten kod to chyba kpina, pisany w jednej linij...
Może ja czegoś nie wiem, co Wy o tym myślicie?

[PHP] pobierz, plaintext 
<?public function __construct() {
global $db;
 
$this->name = 'konfiguracja';
$this->title = 'Konfiguracja serwisu';
 
$db->query("SELECT tresc FROM konfiguracja WHERE nazwa='blokada'");
$res = $db->getNextRow();
 
if ($res['tresc'] == 'tak') {
echo '';
$this->checked = 'checked';
} else {
echo '';
$this->checked = '';
}
 
switch ($_GET['do']) {
default:
$this->show();
break;
}
}
 
public function show() {
global $db;
 
$db->query("SELECT * FROM " . $this->name . " WHERE id_jezyk=%d", $_SESSION['id_jezyk']);
$r = $db->res();
 
for ($i = 0; $i < count($r); $i++) {
if ($r[$i]['id_fraza'] == 1)
$keyw = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 2)
$opis = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 3)
$tytul = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 4)
$email = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 5)
$szer = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 6)
$wys = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 7)
$s1 = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 8)
$s2 = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 9)
$konto = $r[$i]['tresc'];
 
if ($r[$i]['id_fraza'] == 10)
$licznik = $r[$i]['tresc'];
 
if ($r[$i]['id_fraza'] == 11)
$updateczas = $r[$i]['tresc'];
 
if ($r[$i]['id_fraza'] == 12)
$updateczestosc = $r[$i]['tresc'];
}
 
if (isset($_POST['save'])) { //zapisz do bazy
 
if ($r) {
$db->query("DELETE FROM " . $this->name . " WHERE id_jezyk=%d", $_SESSION['id_jezyk']);
}
 
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 1, $_SESSION['id_jezyk'], $_POST['keyw']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 2, $_SESSION['id_jezyk'], $_POST['opis']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 3, $_SESSION['id_jezyk'], $_POST['tytul']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 4, $_SESSION['id_jezyk'], $_POST['email']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 5, $_SESSION['id_jezyk'], $_POST['szer']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 6, $_SESSION['id_jezyk'], $_POST['wys']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 7, $_SESSION['id_jezyk'], $_POST['szkontakt']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 8, $_SESSION['id_jezyk'], $_POST['s2']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 9, $_SESSION['id_jezyk'], $_POST['konto']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 10, $_SESSION['id_jezyk'], $_POST['licznik']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 11, $_SESSION['id_jezyk'], $_POST['updateczas']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 12, $_SESSION['id_jezyk'], $_POST['updateczestosc']);
 
if ($_POST['blokada'] == 'tak') {
$db->query("UPDATE konfiguracja SET tresc='tak' WHERE nazwa='blokada'");
} else {
$db->query("UPDATE konfiguracja SET tresc='nie' WHERE nazwa='blokada'");
}
 
 
//pobierz meta
$db->query("SELECT * FROM konfiguracja WHERE id_jezyk=%d", $_SESSION['id_jezyk']);
$mres = $db->res();
if ($mres)
foreach ($mres as $d => $b)
$_SESSION['meta'][$b['id_fraza']] = $b['tresc'];
 
header("location:admin.php?module=" . $this->name . "&msg=Zmiany zapisane");
}
else { //pokaz
?>
<h1>Konfiguracja serwisu</h1>
<a href="admin.php?module=pomoc&do=<? echo $this->name; ?>">Pomoc</a>
<form action="" method="post">
<table cellpadding="0" cellspacing="0" style="width:90%">
<tr>
<td style="width:200px">Słowa kluczowe: </td>
<td><input type="text" class="pole" size="60" name="keyw" value="<? echo $keyw; ?>" /></td>
</tr>
<tr>
<td >Opis strony: </td>
<td><input type="text" class="pole" size="60" name="opis" value="<? echo $opis; ?>" /></td>
</tr>
<tr>
<td >Tytuł strony: </td>
<td><input type="text" class="pole" size="60" name="tytul" value="<? echo $tytul; ?>" /></td>
</tr>
<tr>
<td >E-mail kontaktowy: </td>
<td><input type="text" class="pole" size="60" name="email" value="<? echo $email; ?>" /></td>
</tr>
<tr>
<td >Licznik: </td>
<td><input type="text" class="pole" size="60" name="licznik" value="<? echo $licznik; ?>" /></td>
</tr>
<tr>
<td >Dane do wpłat: (bank, numer, dane odbiorcy) </td>
<td><textarea style="width: 379px; height: 69px;" name="konto"><? echo $konto; ?></textarea></td>
</tr>
<tr>
<td >Szybki kontakt: </td>
<td><textarea name="szkontakt" /><? echo $s1; ?></textarea></td>
</tr>                    
<tr>
<td >Update produktów - ostatni: </td>
<td><input type="text" class="pole" size="60" name="updateczas" value="<? echo $updateczas; ?>" /></td>
</tr>
<tr>
<td >Update produktów - częstość (min): </td>
<td><input type="text" class="pole" size="60" name="updateczestosc" value="<? echo $updateczestosc; ?>" /></td>
</tr>
 
</table>
<input type="submit" class="go" name="save" value="Zapisz" />
<input type="button" class="go" name="cancel" value="Anuluj" onclick="window.location='admin.php'" />
</form>
[PHP] pobierz, plaintext 

Ten post edytował Adi32 13.07.2012, 08:45:11

[Sephirus]

Ło ja Cię kręcę ale po kolei...

[PHP] pobierz, plaintext 
global $db; // hmm nie słyszeli o przekazywaniu argumentów...
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
$db->query("SELECT tresc FROM konfiguracja WHERE nazwa='blokada'"); // wydajność, wydajność i jeszcze raz wydajność 
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
if ($res['tresc'] == 'tak') {
echo '';                    // To echo jest tutaj po to by?
$this->checked = 'checked';
} else { 
echo '';                   // To echo jest tutaj po to by?
$this->checked = '';
}
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
 
switch ($_GET['do']) { // rozumiem że to jest dodane na zaś?
default:
$this->show();
break;
}
}
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
for ($i = 0; $i < count($r); $i++) { // a już myślałem że wiedzą co to switch(...)...
if ($r[$i]['id_fraza'] == 1)
$keyw = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 2)
$opis = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 3)
$tytul = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 4)
$email = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 5)
$szer = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 6)
$wys = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 7)
$s1 = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 8)
$s2 = $r[$i]['tresc'];
if ($r[$i]['id_fraza'] == 9)
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
$db->query("UPDATE konfiguracja SET tresc='tak' WHERE nazwa='blokada'"); // brak konsekwencji - raz "$this->name" raz "konfiguracja" ...
[PHP] pobierz, plaintext 

Skąd Ty to wziąłeś? Nie mów, że zapłaciłeś tym profesjonalistom?

[Adi32]

To jest kod głównego programisty w firmie w której pracuję...
Nie powiem jaka to firma, ale powiem, że musze stąd uciec...

Ten post edytował Adi32 13.07.2012, 08:55:15

[Sephirus]

Znaczy powiem tak, żeby nie było - luk bezpieczeństwa jakichś mega tu nie ma - ale jest nieład, niedbalstwo i co najgorsze - choć może tu nie najistotniejsze - brak pomyślunku jeśli chodzi o wydajność...

Może się już mu nie chce...

Co do wydajności to to:

[PHP] pobierz, plaintext 
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 1, $_SESSION['id_jezyk'], $_POST['keyw']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 2, $_SESSION['id_jezyk'], $_POST['opis']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 3, $_SESSION['id_jezyk'], $_POST['tytul']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 4, $_SESSION['id_jezyk'], $_POST['email']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 5, $_SESSION['id_jezyk'], $_POST['szer']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 6, $_SESSION['id_jezyk'], $_POST['wys']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 7, $_SESSION['id_jezyk'], $_POST['szkontakt']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 8, $_SESSION['id_jezyk'], $_POST['s2']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 9, $_SESSION['id_jezyk'], $_POST['konto']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 10, $_SESSION['id_jezyk'], $_POST['licznik']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 11, $_SESSION['id_jezyk'], $_POST['updateczas']);
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 12, $_SESSION['id_jezyk'], $_POST['updateczestosc']);
[PHP] pobierz, plaintext 

Można dać w jednym zapytaniu... itd... itp...

No i global $db :/ to mi nie da zasnąć...

Ten post edytował Sephirus 13.07.2012, 08:57:49

[Adi32]

Może się już mu nie chce...

Szybko zgadłeś, ale edycja i rozwijanie takiego kodu to koszmar...

Edit. Zauważ, że przy każdym zapisie wszystkie pola są kasowane i dodawane na nowo.


Ten post edytował Adi32 13.07.2012, 08:59:26

[m44]

Czepiacie się, działać działa!

[redeemer]

Znaczy powiem tak, żeby nie było - luk bezpieczeństwa jakichś mega tu nie ma.

Zależy co to za obiekt $db i jak wygląda metoda query. Jeśli argumenty tej metody nie są escape'owane to mamy SQL injection.

[PHP] pobierz, plaintext 
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 2, $_SESSION['id_jezyk'], $_POST['opis']);
[PHP] pobierz, plaintext 

[Adi32]

Zgadza się, ale pocieszenie jest takie, że niezabezpieczone zapytanie są tylko w panelu administracyjnym.

[Sephirus]

@redeemer - Masz całkowitą rację

po prostu biorąc ten kod:

[PHP] pobierz, plaintext 
$db->query("INSERT INTO " . $this->name . " (id_fraza,id_jezyk,tresc) VALUES (%d,%d,'%s')", 2, $_SESSION['id_jezyk'], $_POST['opis']);
[PHP] pobierz, plaintext 

Założyłem że kolejne argumenty są automatycznie zabezpieczane ale w sumie jak teraz popatrzeć to chyba są walone wprost ... a to jest lipne strasznie...

Zgadza się, ale pocieszenie jest takie, że niezabezpieczone zapytanie są tylko w panelu administracyjnym.

heh Może to i jakieś pocieszenie ale i tak marne zabezpieczanie powinno być w krwi - zawsze - wszędzie. Nie można myśleć na zasadzie - to jest panel administracyjny a do niego dostęp mam tylko ja, a ja nie mam zamiaru robić SQL-inj. to mogę nie zabezpieczyć - złe nawyki

[redeemer]

Jak napisał Sephirus to że jest to admin, nie ma znaczenia. Można też założyć, że ten obiekt oraz metoda query jest wykorzystywana również w frontendzie.

[Niktoś]

Nic dodać, nic ująć-kod z optymalnością mało wspólnego.Choć programuje w ASP.NET to nie chwaląc się ,pewnie zrobiłbym to lepiej.Brak pomysłu albo lenistwo -to nie pisał profesjonalista-albo mu się nie chciało i pisał to od ręki wyłączając przy tym część swoich szarych komórek.

[PHP] pobierz, plaintext 
switch ($_GET['do']) {
default:
$this->show();
break;
}
[PHP] pobierz, plaintext 

Tego to normalnie nie rozumiem-czy to ma być instrukcja wyboru.Jakiś hack and trick?Nie wystarczyło dać if-a?

[PHP] pobierz, plaintext 
    if ($res['tresc'] == 'tak') {
    echo ''; // To echo jest tutaj po to by?questionmark.gif
    $this->checked = 'checked';
    } else {
    echo ''; // To echo jest tutaj po to by?questionmark.gif
    $this->checked = '';
    }
[PHP] pobierz, plaintext 

Gdzie tutaj logika?Jeśli warunek spełniony wyświetl nic jeśli nie, także wyświetl nic

Poza tym o ile to możliwe można poniższe liniki zrobić na poziomie bazy danych:

[PHP] pobierz, plaintext 
if ($_POST['blokada'] == 'tak') {
$db->query("UPDATE konfiguracja SET tresc='tak' WHERE nazwa='blokada'");
} else {
$db->query("UPDATE konfiguracja SET tresc='nie' WHERE nazwa='blokada'");
}
[PHP] pobierz, plaintext 

Ustawić default(domyślną wartość) dla kolumny treść na 'nie' i jeśli warunek if ($_POST['blokada'] == 'tak') zostałby spełniony zrobić tylko update , else nie musiałoby w ogóle istnieć.
Przykład:

[PHP] pobierz, plaintext 
if ($_POST['blokada'] == 'tak') {
$db->query("UPDATE konfiguracja SET tresc='tak' WHERE nazwa='blokada'");
}
[PHP] pobierz, plaintext 

Else nie potrzebne gdyż domyślna wartość kolumny tresc jest ustawiona na nie i nie trzeba jej zmieniać.

Ten post edytował Niktoś 13.07.2012, 10:44:53

[Adi32]

@Niktoś - ja to wiem i szczerze załamuje mnie to.
To co przedstawiłem to tylko kawałeczek. Ja muszę przy tym pracować.
Tutaj ludzie uważają, że to jest programowanie obiektowe, mówię nie - "przecież są klasy"...
Pyta mnie - "split deprecated" - mowie przestarzała, użyj preg_math - "przecież preg_math służy do validacji"... (po aktualizacji PHP)

Ten post edytował Adi32 13.07.2012, 10:54:44

[skowron-line]

@Adi32 czepiasz się każdy może mieć gorszy dzień buahahaha

[Tuminure]

użyj preg_math - "przecież preg_math służy do validacji"

To jakaś nowa matematyczna funkcja ?

Mam nadzieję, że nie starasz się zbyt mocno nad rozwijaniem czegoś takiego .

[Adi32]

To jakaś nowa matematyczna funkcja ?

Mam nadzieję, że nie starasz się zbyt mocno nad rozwijaniem czegoś takiego .

Kiedy tu trafiłem pomyślałem, że użyje doświadczenia jakie mam aby to "rozwinąć" ale nie, spotkało się to z oporem gdyż tek kto ma ciepłą posadkę i nikogo nad sobą nie potrzebuje sie uczyć.

Wkleiłem tylko niewielki kawałek kodu nic nie prezentujący żeby nikt się nie przyczepił ale spróbujcie sobie wyobrazić jak wygląda kod sklepu...
Proponowałem Zenda - zapomnij ... "jak chcesz to sam zrób coś prostszego", kto coś łapie wie, że mija się to z celem...

[redeemer]

W takim razie szukaj innej firmy, bo niczego się nie nauczysz.

[everth]

@Adi32
Jesteś romantykiem. Rzeczywistość jest dużo gorsza niż to co podałeś

[Adi32]

W takim razie szukaj innej firmy, bo niczego się nie nauczysz.

Szukam... Mam nawet zaproszenia na kilka rozmów.

@Adi32
Jesteś romantykiem. Rzeczywistość jest dużo gorsza niż to co podałeś

A co dokładniej przez to rozumiesz? Ja rozumiem, że mogą być gorsze kody, ale chyba nie w "profesjonalnej firmie"...

[Niktoś]

Jak to jest Jego obiektówka, to wklej jakiś kod strukturalny, chciałbym wiedzieć jak wygląda.

[Adi32]

Jak to jest Jego obiektówka, to wklej jakiś kod strukturalny, chciałbym wiedzieć jak wygląda.

Żeby ukazać jego moc musiałbym wkleić całe 500 linij pliku top.
Boje się w ogóle tam zaglądać...

Ten post edytował Adi32 13.07.2012, 11:17:00