[aur-dev] [PATCH 2/4] rpc.php overhaul

Dan McGee dan at archlinux.org
Tue Apr 12 01:15:47 EDT 2011


* Mark things static in the class rather than use a constructor every
  single invocation of the service.
* Don't call mysql_real_escape_string() before we even have a database
  connection, and don't do work in the database if we don't need to.
* Formatting consistency fixups in a few places.
* Add new process_query() helper function; use this instead of
  copy-pasted code in all of the RPC method calls.
* Remove the escaping code meant to fix FS#15526, introduced in commit
  4d1eb4dd7ac631. It broke more than it solved, only fixed the output in
  one of three RPC calls (and who knows what the web interface then also
  does), and proper encoding should be done at the database level rather
  than up here.

Signed-off-by: Dan McGee <dan at archlinux.org>
---

For more on the busted escape patch (https://bugs.archlinux.org/task/15526)
being totally broken, see the following case study:
* http://aur.archlinux.org/packages.php?ID=13711
* http://aur.archlinux.org/rpc.php?type=info&arg=stardict-ldaf
* http://aur.archlinux.org/rpc.php?type=search&arg=stardict-ldaf

 web/lib/aurjson.class.php |  106 ++++++++++++++++-----------------------------
 1 files changed, 37 insertions(+), 69 deletions(-)

diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
index bc26826..a22be62 100644
--- a/web/lib/aurjson.class.php
+++ b/web/lib/aurjson.class.php
@@ -14,23 +14,12 @@ include_once("aur.inc");
  **/
 class AurJSON {
     private $dbh = false;
-    private $exposed_methods = array();
-    private $fields = array();
-
-    /**
-     * Initialize methods and database fields.
-     **/
-    public function __construct() {
-        $this->exposed_methods = array('search', 'info', 'msearch');
-
-	$this->fields = array(
-	    'Packages.ID', 'Name', 'Version', 'CategoryID',
-	    'Description', 'URL', 'CONCAT("' .
-	    mysql_real_escape_string(URL_DIR) .
-	    '", Name, "/", Name, ".tar.gz") AS URLPath', 'License',
-	    'NumVotes', '(OutOfDateTS IS NOT NULL) AS OutOfDate'
-	);
-    }
+    private static $exposed_methods = array('search', 'info', 'msearch');
+    private static $fields = array(
+        'Packages.ID', 'Name', 'Version', 'CategoryID',
+        'Description', 'URL', 'License',
+        'NumVotes', '(OutOfDateTS IS NOT NULL) AS OutOfDate'
+    );
 
     /**
      * Handles post data, and routes the request.
@@ -44,12 +33,12 @@ class AurJSON {
         }
 
         // do the routing
-        if ( in_array($http_data['type'], $this->exposed_methods) ) {
+        if ( in_array($http_data['type'], self::$exposed_methods) ) {
             // set up db connection.
             $this->dbh = db_connect();
 
             // ugh. this works. I hate you php.
-            $json = call_user_func(array(&$this,$http_data['type']),
+            $json = call_user_func(array(&$this, $http_data['type']),
                 $http_data['arg']);
 
             // allow rpc callback for XDomainAjax
@@ -76,10 +65,10 @@ class AurJSON {
      * @param $msg The error string to return
      * @return mixed A json formatted error response.
      **/
-    private function json_error($msg){
+    private function json_error($msg) {
         // set content type header to app/json
         header('content-type: application/json');
-        return $this->json_results('error',$msg);
+        return $this->json_results('error', $msg);
     }
 
     /**
@@ -88,10 +77,29 @@ class AurJSON {
      * @param $data The result data to return
      * @return mixed A json formatted result response.
      **/
-    private function json_results($type,$data){
+    private function json_results($type, $data) {
         return json_encode( array('type' => $type, 'results' => $data) );
     }
 
+    private function process_query($type, $query) {
+        $result = db_query($query, $this->dbh);
+
+        if ( $result && (mysql_num_rows($result) > 0) ) {
+            $search_data = array();
+            while ( $row = mysql_fetch_assoc($result) ) {
+                $name = $row['Name'];
+                $row['URLPath'] = URL_DIR . $name . "/" . $name . ".tar.gz";
+                array_push($search_data, $row);
+            }
+
+            mysql_free_result($result);
+            return $this->json_results($type, $search_data);
+        }
+        else {
+            return $this->json_error('No results found');
+        }
+    }
+
     /**
      * Performs a fulltext mysql search of the package database.
      * @param $keyword_string A string of keywords to search with.
@@ -105,24 +113,12 @@ class AurJSON {
         $keyword_string = mysql_real_escape_string($keyword_string, $this->dbh);
         $keyword_string = addcslashes($keyword_string, '%_');
 
-        $query = "SELECT " . implode(',', $this->fields) .
+        $query = "SELECT " . implode(',', self::$fields) .
             " FROM Packages WHERE " .
             "  ( Name LIKE '%{$keyword_string}%' OR " .
             "    Description LIKE '%{$keyword_string}%' )";
-        $result = db_query($query, $this->dbh);
 
-        if ( $result && (mysql_num_rows($result) > 0) ) {
-            $search_data = array();
-            while ( $row = mysql_fetch_assoc($result) ) {
-                array_push($search_data, $row);
-            }
-
-            mysql_free_result($result);
-            return $this->json_results('search', $search_data);
-        }
-        else {
-            return $this->json_error('No results found');
-        }
+        return $this->process_query('search', $query);
     }
 
     /**
@@ -131,7 +127,7 @@ class AurJSON {
      * @return mixed Returns an array of value data containing the package data
      **/
     private function info($pqdata) {
-        $base_query = "SELECT " . implode(',', $this->fields) .
+        $base_query = "SELECT " . implode(',', self::$fields) .
             " FROM Packages WHERE ";
 
         if ( is_numeric($pqdata) ) {
@@ -147,26 +143,9 @@ class AurJSON {
             $query_stub = sprintf("Name=\"%s\"",
                 mysql_real_escape_string($pqdata));
         }
+        $query = $base_query . $query_stub;
 
-        $result = db_query($base_query.$query_stub, $this->dbh);
-
-        if ( $result && (mysql_num_rows($result) > 0) ) {
-            $row = mysql_fetch_assoc($result);
-            mysql_free_result($result);
-            foreach($row as $name => $value) {
-                $converted = utf8_encode($value);
-                if ($converted != "") {
-                    $row[$name] = $converted;
-                }
-                else {
-                    $row[$name] = "[PKGBUILD error: non-UTF8 character]";
-                }
-            }
-            return $this->json_results('info', $row);
-        }
-        else {
-            return $this->json_error('No result found');
-        }
+        return $this->process_query('info', $query);
     }
 
     /**
@@ -176,25 +155,14 @@ class AurJSON {
      **/
     private function msearch($maintainer) {
         $maintainer = mysql_real_escape_string($maintainer, $this->dbh);
-        $fields = implode(',', $this->fields);
+        $fields = implode(',', self::$fields);
 
         $query = "SELECT Users.Username as Maintainer, {$fields} " .
             " FROM Packages, Users " .
             "        WHERE Packages.MaintainerUID = Users.ID AND " .
             "              Users.Username = '{$maintainer}'";
-        $result = db_query($query, $this->dbh);
 
-        if ( $result && (mysql_num_rows($result) > 0) ) {
-            $packages = array();
-            while ( $row = mysql_fetch_assoc($result) ) {
-                array_push($packages, $row);
-            }
-            mysql_free_result($result);
-            return $this->json_results('msearch', $packages);
-        }
-        else {
-            return $this->json_error('No results found');
-        }
+        return $this->process_query('msearch', $query);
     }
 }
 
-- 
1.7.4.4



More information about the aur-dev mailing list