7
\$\begingroup\$

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

<?php
    header("Access-Control-Allow-Origin:*");

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

    $params = $_GET;
    $user = 'root'; // Change this to username
    $password = ''; // Put your password here. My home database doesn't have any password (is empty).

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

    $context = stream_context_create(
        array(
            'http' => 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 < time()){
                if($res['version'] == '1'){
                    $base = $res['IP'].'/admin.cgi?mode=viewxml&user=admin&pass='.$res['PASS'];
                    // Well i will change this crazy method with something non password secured pretty soon
                }else{
                    $base = $res['IP'].'/stats?sid=1';
                    // As per shoutcast v2 APIs !! :D
                }
                $content = file_get_contents($base, false, $context)or die("Unable to Load resourse");
                $dom = new DOMDocument();
                $dom->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='.time().' where ID=\''.$params['id']. '\';';
                mysql_query($query,$con)or die(mysql_error());
                mysql_close($con);
            }
            unset( $res['IP'] );
            unset( $res['PASS'] );
            echo json_encode($res);
        }
    }else if($_params['act']== 's'){
        // To search for a specific keyword
        $queryString = $params['q'];
    }else if($_params['act'] == 'l'){
        // To feed list to the applet
        $cat = $params['c'];
        // Categories are various lists depending on genre languages etc...
    }
?>

I was wondering if making a phpSocket server would be better for this purpose as sockets will reduce the demand of required MySQL connections to be opened.

Or

Is my current method the best solution?

\$\endgroup\$
  • 2
    \$\begingroup\$ Well first I'd suggest you ditch the mysql_* functions. :) Both PHP and MySQL have moved on a lot since the early days and the mysql_* set of functions really just don't cut it anymore. I'd strongly advise switching to mysqli or PDO. \$\endgroup\$ – GordonM Feb 5 '12 at 11:41
  • \$\begingroup\$ Also, your code is wide open to SQL injection because you do no filtering or validating of the user input (other than assigning $_GET to $params). mysqli and PDO both support a mechanism called prepared statements that allow automatic escaping of variables when they're injected into a SQL statement. (I would still implement some proper validation of user input as well before passing it to the DB though. You can never be too careful) \$\endgroup\$ – GordonM Feb 5 '12 at 11:43
  • \$\begingroup\$ @GordonM thanks for the heads up i will fix it as soon as possible :) \$\endgroup\$ – ShrekOverflow Feb 5 '12 at 13:27
  • \$\begingroup\$ Its been ages since i have actually written something on PHP so it feels kind of confusing now , anyways still socket or Multi Script ? which is better ? also with sockets i can remove the MYSQL need totally by storing all the values into RAM . \$\endgroup\$ – ShrekOverflow Feb 5 '12 at 13:37
  • 1
    \$\begingroup\$ Switch to PDO! I'd also recommend breaking up some code logically into functions. You have too many if statements and nested if statements and it makes it hard to follow the code. \$\endgroup\$ – Cobby May 16 '12 at 2:47
6
\$\begingroup\$
  1. 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.

  2. $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.

  3. $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?)

  4. $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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.