schlampig gecodet?

Seite 1 von 2 - Forum: Coding Stuff auf overclockers.at

URL: https://www.overclockers.at/coding-stuff/schlampig_gecodet_144546/page_1 - zur Vollversion wechseln!

tomstig schrieb am 22.06.2005 um 18:53

Da ich für eine Party eine Gästeliste brauchte, die von mehreren editiert werden kann, bastelte ich eine mit PHP (+MySQL).

Der Code enstand in ca. 30-45min.

Ist der folgende Code gut bzw. in Ordnung oder ist der einfach unter aller Sau und nur schnell hingfetzt?

Code: PHP
<html>
<head>
<title>Gästeliste</title>
<style type="text/css">
body, th, td, a{
	color: #000;
	font-family: Verdana;
	font-size: 10pt;
	padding: 5px;
}

td{
	border: 1px solid #000;
}

a:link, a:visited{
	text-decoration: underline;
}

a:hover{
	text-decoration: none;
}

input.none{
	border: none;
}
</style>
</head>
<body>
<?php

error_reporting(E_ALL);

$db = mysql_connect("localhost", "root", "");
mysql_select_db("test", $db);

if( isset($_GET['submit']) )
	$sql = "INSERT INTO guestlist SET name='" . $_GET['name'] . "', fix=" . ( $_GET['fix'] == 1 ? 1 : 0 ) . ", time_added='" . time() . "'";	
elseif( isset($_GET['delete']) )
	$sql = "DELETE FROM guestlist WHERE guest_id=" . $_GET['delete'];	
elseif( isset($_GET['changed_name']) )
	$sql = "UPDATE guestlist SET name='" . $_GET['changed_name'] . "' WHERE guest_id=" . $_GET['guest_id'];
elseif( isset($_GET['fix']) ){
	$sql = "SElECT fix FROM guestlist WHERE guest_id=" . $_GET['fix'];
	$query = mysql_query($sql);
	$fix = mysql_result($query, 0);
	
	$sql = "UPDATE guestlist SET fix=" . ($fix == 0 ? 1 : 0) . " WHERE guest_id=" . $_GET['fix'];
}
	
if( !empty($sql) ){
	mysql_query($sql);
	echo '<script type="text/javascript">window.location.href=\'guestlist.php\';</script>';
}
if( isset($_GET['order']) )
	echo '<a href="' . $_SERVER['PHP_SELF'] . '">Wieder umordnen...</a><br /><br />';
	
echo '<form name="form2" action="' . $_SERVER['PHP_SELF'] . '" method="GET"><input type="text" name="name" value"G&aumlstename"><input type="checkbox" name="fix" value="1" title="Kommt dieser Gast sicher?"><input type="submit" name="submit" value="Gast eintragen"></form>';
echo '<script type="text/javascript">document.getElementsByName(\'name\')[0].focus();</script>';

$sql = "SELECT * FROM guestlist ORDER BY " . (isset($_GET['order']) ? $_GET['order'] : 'time_added' ) . " ASC";
$query = mysql_query($sql);

if( mysql_num_rows($query) != 0){
	echo '<table>';
	echo '<tr><th>#</th><th><a href="' . $_SERVER['PHP_SELF'] . '?order=name">Name</a></th><th><a href="' . $_SERVER['PHP_SELF'] . '?order=fix">Kommt fix?</a></th><th>Optionen</th><th>Eingetragen am</th></tr>';
	$counter = 0;
	while($data = mysql_fetch_array($query)){
		echo '<form name="formular" action="' . $_SERVER['PHP_SELF'] . '" method="GET">
<tr>
<td>' . ($counter+1) . '</td>
<td><input type="hidden" name="guest_id" value="' . $data['guest_id'] . '"><input type="text" name="changed_name" value="' . $data['name'] . '" class="none"></td><td>' . ( $data['fix'] == 1 ? 'Ja' : 'Nein' ) . '</td>
<td><a href="' . $_SERVER['PHP_SELF'] . '?fix=' . $data['guest_id'] . '">[ ' . ($data['fix']==1 ? 'de' : '') . 'fixieren ]</a>
 <a href="' . $_SERVER['PHP_SELF'] . '?delete=' . $data['guest_id'] . '">[ löschen ]</a>
 <a href="#" onclick="document.getElementsByName(\'formular\')[' . $counter . '].submit()">[ Name ändern ]</a></td>
<td>' . date('d.m.Y, H:i:s', $data['time_added']) . '</td>
</tr></form>';
		$counter++;
	}
	echo '</table>';
}
?>
</body>
</html>


mysql tabelle:
CREATE TABLE `guestlist` (
  `guest_id` int(11) NOT NULL auto_increment,
  `name` varchar(255) NOT NULL default '',
  `fix` int(11) NOT NULL default '0',
  `time_added` varchar(255) NOT NULL default '',
  PRIMARY KEY  (`guest_id`)
) ENGINE=MyISAM AUTO_INCREMENT=26 ;

funka schrieb am 22.06.2005 um 19:33

if its stupid but works its not stupid

it does the job so let it do the job

never change a running system

willst noch mehr kluge sprueche? ;)

moidaschl schrieb am 22.06.2005 um 19:36

Zitat von funka
if its stupid but works its not stupid

it does the job so let it do the job

never change a running system

willst noch mehr kluge sprueche? ;)

i kenns nur unter "never touch a running system"

das einzige vor dem es mich ein bissl graust is deine 3km lange echo anweisung, die könntest irgendwo abschneiden und darunter schön einrichten.

also zb

Code:
echo "irgendwas das eindeutig zu
      lang ist für eine zeile";

wenn ein tag zu lang is würd ich ihn nach einem argument kürzen, also so

Code:
echo '<input name="hans" value="hans123"
             size="20" type="text">\n';

tomstig schrieb am 22.06.2005 um 19:55

Zitat von funka
if its stupid but works its not stupid

it does the job so let it do the job

never change a running system

willst noch mehr kluge sprueche? ;)

"If a code was hard to write it should also be hard to unterstand" - mein fav ;)

naja trotzdem - verbesserungsvorschläge kann man sich doch immer holen ;)

@moidaschl: in meinem editor war das lange echo eh nur in einer zeile :D
aber ich wollte hier keinen horizontalen scrollbalken erzwingen ;)

watchout schrieb am 22.06.2005 um 19:58

funka hat zwar recht - ich würd jedoch aufpassen weil das Script extrem unsicher ist.

funka schrieb am 22.06.2005 um 20:00

watch hat recht

wegen code wie solchem gibs "features" wie magic_quotes u.ä.

kleinerChemiker schrieb am 22.06.2005 um 21:31

oder mysql_escape_string()

tomstig schrieb am 22.06.2005 um 21:34

Zitat von watchout
funka hat zwar recht - ich würd jedoch aufpassen weil das Script extrem unsicher ist.

hmm... auf sicherheit hab ich nicht viel wert gelegt, aber kannst du mir vielleicht sagen, was so unsicher ist?

watchout schrieb am 22.06.2005 um 21:37

du übernimmst (mehrmals) direkt werte die vom user kommen unquoted in einen query. mysql injection wird möglich.

Obermotz schrieb am 22.06.2005 um 21:50

Ich würds auch noch besser absichern, alle haben es drauf abgesehen, deine Party-Gästeliste zu modifizieren! ;)

scnr

watchout schrieb am 22.06.2005 um 22:00

Wie du schon andeutest ist eine Party-Gästeliste eher weniger wichtig - deswegen wird sie auch wohl kaum auf einem eigenen Server liegen - Was denkst du wie gross die Freude is wenn du wegen einer kleinen besch. Gästeliste einen kompletten Server geschrottet hast, wo vielleicht auch noch wichtige Firmendaten oben sind...

@Oper8or: Verwarnung - scnr ;)

tomstig schrieb am 22.06.2005 um 22:39

Zitat von watchout
du übernimmst (mehrmals) direkt werte die vom user kommen unquoted in einen query. mysql injection wird möglich.

was ist unquoted :confused:
wie behandelt man sonst die werte, wenn man sie mit $_GET ausliest?

kleinerChemiker schrieb am 22.06.2005 um 22:41

mysql_escape_string($_GET[''])

watchout schrieb am 22.06.2005 um 22:42

zumindest mysql_escape_string($_GET[...])

edit: :(

Ringding schrieb am 22.06.2005 um 22:42

Google for sql injection.



overclockers.at v4.thecommunity
© all rights reserved by overclockers.at 2000-2025