Wat kan beter aan dit script?

[DarkFeather]

Geplaatst door JPeetje
Raad eens waar hij die grote stukken script vandaan heeft ?

Ik heb geen idee

Als voor 't script, je hebt daar een hele aparte functie in zitten:

function slashes($tekst)
{
  if(get_magic_quotes_gpc() == FALSE)
  {
  $tekst = addslashes($tekst);
  }
  return $tekst;
  }
}

Die functie lijkt errug veel op die ik gebruik ... zelfs de naam is 't zelfde ...

Beter goed gejat dan slecht bedacht!

Hou er wel rekening mee dat die gpc niet zomaar leuke lettertjes zijn aan 't einde van de functie, dat is waar ze op slaan: GET, POST en COOKIE waarden.

Dat had ik al opgezocht, maar nog een extra uitleg kan geen kwaad !

Enkele aanmerkingen op je script, dit stukje:

include("connect.php");

if(get_magic_quotes_gpc() == FALSE)
    {
    $id = addslashes(((int)$_GET['id']));
    }
else
  {
  $id = ((int)$_GET['id']);
  }

Kan vervangen worden door:

include("connect.php");

$id = slashes($_GET["id"]);

Aangezien de functie er toch al is. Maar het kan nog korter, aangezien je er toch een integer, oftewel een geheel getal, van maakt, hoef je je geen zorgen te maken over die slashes. Dus simpelweg:

$id = ((int) $_GET["id"]);

is genoeg.

Had er nog niet zo heel diep over nagedacht.

Bij

    echo "Er is een probleem tijdens het selecteren van de informatie uit de database";

zou je wel een mysql_error() toe mogen voegen, maar is niet vereist.

Okee.

Dit daarentegen is wel fout:

    if(isset($_POST['bewerk']) == FALSE)

In de else { die daarna volgt, vraag je met $_POST maar liefst een id,naam en tekst op. Ik zie nergens een controle waarbij je checkt of deze waarden ook bestaan.
Verder gebruik je voor $id ook slashes($_POST["id"]);
Hierdoor is het niet zeker dat $id een integer is, aangezien iedereen een HTML formulier op zijn eigen pc kan maken die het formulier naar jouw site verwijst:

<form action="jouw_website" method="post">

Als men hierbij voor 'id' een tekstveld maakt met de waarde 'blaat', dan komt je query er als volgt uit te zien:

UPDATE tabel SET naam = 'naam', tekst = 'tekst' WHERE id = blaat

Dat "id = blaat" stukje zal een SQL error opleveren, doordat er geen quotes om "blaat" heen staan. Zou je op $_POST["id"] een (int) toepassen, dan wordt "blaat" veranderd in een getal, dit getal is 0, doordat PHP er gewoonweg niets van kan maken

0 komt dan niet voor in de database, maar de query zal dan geen error opleveren.

Dus ik moet van dit:

$id = slashes($_POST['id']);

dit maken

$id = ((int)$_POST['id'])

En ook dit ertussen voegen?

if(isset($_POST['id']) == FALSE)
{
  echo "Er is geen pagina opgegeven!";
}

if(isset($_POST['naam']) == FALSE)
{
  echo "Er is geen naam opgegeven!";
  die();
}

if(isset($_POST['tekst']) == FALSE)
{
  echo "Er is geen tekst doorgegeven!";
 die();
}

En dat zou ik dan voor dit stuk moeten plaatsen:

    $id = ((int)$_POST['id']);
    $naam = slashes($_POST['naam']);
    $tekst = slashes($_POST['tekst']);
    $query = "UPDATE " . $tabel . " SET naam = '" . $naam . "', tekst = '" . $tekst . "' WHERE id = " . $id; 
    $result = @mysql_query($query);

Verder heb ik alleen nog een paar performance (snelheid) aanmerkingen, als je die wil horen post je 't maar

Laat maar horen!

    $naam = stripslashes($rij->naam);

Hm ... of dat nodig is weet ik zonet nog niet ... hangt er vanaf hoe je 't in de database invoert. Als dat echt met \' of \\ of \" in de database staat, dan moet je stripslashes() toepassen, wil je een fatsoenlijk resultaat. Anders niet.

Het wordt indd met addslashes in de database gezet (zie ook bewerk.php).

Je beveiliging in verwijder.php is trouwens niet al te best. Hij controleert namelijk alleen of $id gelijk is aan 1 als $_POST["verwijder"] niet bestaat. Als deze wel bestaat maakt het kennelijk niet meer uit. Wederom, ik zet een HTML formulier op m'n eigen pc, verwijs deze door naar jouw script, en voila, de rij met id 1 is verwijderd

En hoe kan ik dat verbeteren? En heb ik ook niet zo'n soort beveiligingsfout in bewerk.php?

De ordening in beide scripts is trouwens niet slecht, houden zo !

Dank je. Die heb ik gewoon van masterprut afgekeken (en die zal het vast wel weer van jou hebben )

// Edit 2: Dat smilie limiet per post mag wel versteld worden naar 15 of 20 ...

indd, die mag wel omhoog!!!

 

[JPeetje]

Geplaatst door DarkFeather
Ik heb geen idee

Van mij dus

Dus ik moet van dit:

$id = slashes($_POST['id']);

dit maken

$id = ((int)$_POST['id'])

Helemaal juist.

En ook dit ertussen voegen?

if(isset($_POST['id']) == FALSE)
{
  echo "Er is geen pagina opgegeven!";
}

if(isset($_POST['naam']) == FALSE)
{
  echo "Er is geen naam opgegeven!";
  die();
}

if(isset($_POST['tekst']) == FALSE)
{
  echo "Er is geen tekst doorgegeven!";
 die();
}

Allereerst mis je bij de boven if() statement een die();
Verder, waarom zou je al die meldingen geven ? Het is niet jouw schuld dat je bezoekers gaan prutsen met de formulieren, waarom ben jij het hen verplicht een melding te geven ? Ik had gewoon zoiets gedaan:

if((isset($_POST["id"]) == TRUE) AND (isset($_POST["naam"]) == TRUE) AND (isset($_POST["tekst"]) == TRUE))
  {
  // formulier is juist gepost, hier de formulierverwerking
  }
  else
  {
  // formulier is niet gepost of incompleet gepost, toon het formulier
  }

Is ook makkelijker

En dat zou ik dan voor dit stuk moeten plaatsen:

    $id = ((int)$_POST['id']);
    $naam = slashes($_POST['naam']);
    $tekst = slashes($_POST['tekst']);
    $query = "UPDATE " . $tabel . " SET naam = '" . $naam . "', tekst = '" . $tekst . "' WHERE id = " . $id; 
    $result = @mysql_query($query);

Jup :thumb:

Laat maar horen!

Hmz ok, neem het bovenste stukje code bijv. Een variabele neemt een stukje geheugen in van de server.$id, $naam, $tekst zjn eiglijk overbodig. Hoe je dat ook kan doen:

$query = "UPDATE " . $tabel . " SET naam = '" . slashes($_POST["naam"]) . "',tekst = '" . slashes($_POST["tekst"]) . "' WHERE id = " . ((int) $_POST["id"]);

Hierdoor heeft 't script weer 3 variabelen minder, waardoor er in 't geheugen van de server weer meer plek is

Het wordt indd met addslashes in de database gezet (zie ook bewerk.php).

Stop voor de gein als waarde voor naam eens 'iets \ nog iets' in de database (met behulp van je formulier). In de query wordt dit dan \\ met behulp van magic quotes óf addslashes(). Wanneer query's worden uitgevoerd wordt er al een soort stripslashes() op ze uitgevoert, deze \\ wordt als \ genoteerd in de database. Wanneer je het ophaalt zul je dan ook 'iets \ nog iets' krijgen. stripslashes() is dus niet nodig

En hoe kan ik dat verbeteren? En heb ik ook niet zo'n soort beveiligingsfout in bewerk.php?

Voor zowel bij de formulierverwerking als bij het weergeven van het formulier zelf moet je checken of $id gelijk is aan 1.
Het belangrijkste is bij de formulierverwerking, in die verwerking wordt de rij immers verwijderd.

Dank je. Die heb ik gewoon van masterprut afgekeken (en die zal het vast wel weer van jou hebben )

Niet helemaal, wel grotendeels