HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

What is better for the code segment, sockets or multiple processes?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thewhatbettersocketssegmentprocessesformultiplecode

Problem

I am trying to fetch information out of SHOUTcast for mutiple users at the same time. Here is my current code:

```
array(
//Required To fool ShoutCast or it will flood with the stream instead
'user_agent' => "GET / HTTP/1.0\r\n"
."Host:117.200.149.48\r\n"
."User-Agent: Mozilla/5.0 (compatible; ShoutCastInfoClass/0.0.2; ".PHP_OS.")\r\n"
."\r\n",
),
)
);

if($params['act'] == 'st'){
mysql_select_db('radio',$con);
$count = mysql_query("select * from info where ID='".$params['id']."'")or die('Error In Query'.mysql_error());

$content = null;
$rows = mysql_num_rows($count);
if( $rows > 0 ){
$res = mysql_fetch_assoc($count);

if($res['TIMESTAMP'] + 30 loadXML($content);
$res['SERVERTITLE'] = htmlspecialchars($dom->getElementsByTagName('SERVERTITLE')->item(0)->textContent);
$res['SONGTITLE'] = htmlspecialchars( $dom->getElementsByTagName('SONGTITLE')->item(0)->textContent );
$res['BITRATE'] = $dom->getElementsByTagName('BITRATE')->item(0)->textContent;
$res['GENRE'] = $dom->getElementsByTagName('SERVERGENRE')->item(0)->textContent;
$res['CONTENT'] = $dom->getElementsByTagName('CONTENT')->item(0)->textContent;

if(strpos($res['CONTENT'],'mpeg')){
$res['CONTENT'] = 'MP3';
}else if(strpos($res['CONTENT'],'aac')){
$res['CONTENT']= 'AAC';
}else{
$res['CONTENT'] = 'OGG';
}
$query='update INFO SET SERVERTITLE="'.$res['SERVERTITLE'].
'", SONGTITLE="'.$res['SONGTITLE'].
'", BITRATE="'.$res['BITRATE'].
'", GENRE="'.$res['GENRE'].
'", CONTENT="'.$res['CONTENT'].
'" , TIMESTAMP='.tim

Solution

-
If you want to use this function you should have buffering:

function failOut($http, $text) {
    header('HTTP/1.x ' . $http);
    die(htmlspecialchars($text));
}


Otherwise it's possible that the script sends out some output (including error messages) and you can't send out any header later. (How do headers work with output buffering in PHP?) Anyway, I don't see any call to this function, so you might be able to remove it.

-

$con = mysql_connect('localhost',$user,$password) or die('Could Not Connect..');


As others already suggested you should use mysqli or PDO. Another note is that you might want to set the HTTP response code to 503 here to save your search engine ranking when the database is down.

-

$params = $_GET; 
...
$count = mysql_query("select * from info where ID='".$params['id']."'")or die('Error In Query'.mysql_error());


As others already mentioned in comments it's not safe against SQL injections attacks. mysql_error() should not be the output of the script. Show only a generic error message and log the details to a server-side log file, don't help attackers. It would also be more user-friendly, most of the users doesn't care nor understand MySQL errors (or at least can't fix them). (How to log errors and warnings into a file?)

-

$dom->getElementsByTagName('BITRATE')->item(0)->textContent


This is in the code at least five times (with different tagname). You could create a function (whose parameter is the tagname) to remove the duplication.

Code Snippets

function failOut($http, $text) {
    header('HTTP/1.x ' . $http);
    die(htmlspecialchars($text));
}
$con = mysql_connect('localhost',$user,$password) or die('Could Not Connect..');
$params = $_GET; 
...
$count = mysql_query("select * from info where ID='".$params['id']."'")or die('Error In Query'.mysql_error());
$dom->getElementsByTagName('BITRATE')->item(0)->textContent

Context

StackExchange Code Review Q#8666, answer score: 6

Revisions (0)

No revisions yet.