diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -809,15 +809,15 @@ } if ($cache_selects) { - $cache_selects = ', '.implode(', ', $cache_selects); + $cache_selects = qsprintf($conn, ', %LQ', $cache_selects); } else { - $cache_selects = ''; + $cache_selects = qsprintf($conn, ''); } if ($cache_joins) { - $cache_joins = implode(' ', $cache_joins); + $cache_joins = qsprintf($conn, '%LJ', $cache_joins); } else { - $cache_joins = ''; + $cache_joins = qsprintf($conn, ''); } return array($cache_selects, $cache_joins, $cache_map, $types_map); diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php --- a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php +++ b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php @@ -111,7 +111,7 @@ $key->getType(), $key->getHash()); } - $where[] = implode(' OR ', $sql); + $where[] = qsprintf($conn, '%LO', $sql); } if ($this->isActive !== null) { diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php --- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php @@ -513,7 +513,6 @@ return 'PhabricatorCalendarApplication'; } - protected function willFilterPage(array $events) { $instance_of_event_phids = array(); $recurring_events = array(); diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -453,7 +453,7 @@ private function loadData() { $table = $this->newResultObject(); - $conn_r = $table->establishConnection('r'); + $conn = $table->establishConnection('r'); $selects = array(); @@ -469,13 +469,13 @@ $this->authors = array_merge($basic_authors, $this->responsibles); $this->reviewers = $basic_reviewers; - $selects[] = $this->buildSelectStatement($conn_r); + $selects[] = $this->buildSelectStatement($conn); // Build the query where the responsible users are reviewers, or // projects they are members of are reviewers. $this->authors = $basic_authors; $this->reviewers = array_merge($basic_reviewers, $this->responsibles); - $selects[] = $this->buildSelectStatement($conn_r); + $selects[] = $this->buildSelectStatement($conn); // Put everything back like it was. $this->authors = $basic_authors; @@ -486,21 +486,35 @@ throw $ex; } } else { - $selects[] = $this->buildSelectStatement($conn_r); + $selects[] = $this->buildSelectStatement($conn); } if (count($selects) > 1) { + $unions = null; + foreach ($selects as $select) { + if (!$unions) { + $unions = $select; + continue; + } + + $unions = qsprintf( + $conn, + '%Q UNION DISTINCT %Q', + $unions, + $select); + } + $query = qsprintf( - $conn_r, + $conn, '%Q %Q %Q', - implode(' UNION DISTINCT ', $selects), - $this->buildOrderClause($conn_r, true), - $this->buildLimitClause($conn_r)); + $unions, + $this->buildOrderClause($conn, true), + $this->buildLimitClause($conn)); } else { $query = head($selects); } - return queryfx_all($conn_r, '%Q', $query); + return queryfx_all($conn, '%Q', $query); } private function buildSelectStatement(AphrontDatabaseConnection $conn_r) { diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -471,28 +471,29 @@ // be worth evaluating is to use "CASE". Another approach is to disable // strict mode for this query. + $default_str = qsprintf($conn, '%s', ''); + $default_int = qsprintf($conn, '%d', 0); + $extra_columns = array( - 'phid' => '""', - 'authorPHID' => '""', - 'status' => '""', - 'priority' => 0, - 'title' => '""', - 'description' => '""', - 'dateCreated' => 0, - 'dateModified' => 0, - 'mailKey' => '""', - 'viewPolicy' => '""', - 'editPolicy' => '""', - 'ownerOrdering' => '""', - 'spacePHID' => '""', - 'bridgedObjectPHID' => '""', - 'properties' => '""', - 'points' => 0, - 'subtype' => '""', + 'phid' => $default_str, + 'authorPHID' => $default_str, + 'status' => $default_str, + 'priority' => $default_int, + 'title' => $default_str, + 'description' => $default_str, + 'dateCreated' => $default_int, + 'dateModified' => $default_int, + 'mailKey' => $default_str, + 'viewPolicy' => $default_str, + 'editPolicy' => $default_str, + 'ownerOrdering' => $default_str, + 'spacePHID' => $default_str, + 'bridgedObjectPHID' => $default_str, + 'properties' => $default_str, + 'points' => $default_int, + 'subtype' => $default_str, ); - $defaults = implode(', ', $extra_columns); - $sql = array(); $offset = 0; @@ -520,9 +521,9 @@ $sql[] = qsprintf( $conn, - '(%d, %Q, %f)', + '(%d, %LQ, %f)', $id, - $defaults, + $extra_columns, $subpriority); $offset++; @@ -531,10 +532,10 @@ foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) { queryfx( $conn, - 'INSERT INTO %T (id, %Q, subpriority) VALUES %LQ + 'INSERT INTO %T (id, %LC, subpriority) VALUES %LQ ON DUPLICATE KEY UPDATE subpriority = VALUES(subpriority)', $task->getTableName(), - implode(', ', array_keys($extra_columns)), + array_keys($extra_columns), $chunk); } diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -237,7 +237,7 @@ $where = $this->buildWhereClause($conn); - $group_column = ''; + $group_column = qsprintf($conn, ''); switch ($this->groupBy) { case self::GROUP_PROJECT: $group_column = qsprintf( @@ -601,10 +601,10 @@ } if (!$subclause) { - return ''; + return qsprintf($conn, ''); } - return '('.implode(') OR (', $subclause).')'; + return qsprintf($conn, '%LO', $subclause); } protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { @@ -736,7 +736,7 @@ return $joins; } - protected function buildGroupClause(AphrontDatabaseConnection $conn_r) { + protected function buildGroupClause(AphrontDatabaseConnection $conn) { $joined_multiple_rows = ($this->hasOpenParents !== null) || ($this->hasOpenSubtasks !== null) || @@ -750,13 +750,13 @@ // task IDs. if ($joined_multiple_rows) { if ($joined_project_name) { - return 'GROUP BY task.phid, projectGroup.dst'; + return qsprintf($conn, 'GROUP BY task.phid, projectGroup.dst'); } else { - return 'GROUP BY task.phid'; + return qsprintf($conn, 'GROUP BY task.phid'); } - } else { - return ''; } + + return qsprintf($conn, ''); } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -663,9 +663,9 @@ if ($sql) { queryfx( $conn_w, - 'INSERT INTO %T (userID, token) VALUES %Q', + 'INSERT INTO %T (userID, token) VALUES %LQ', $table, - implode(', ', $sql)); + $sql); } } diff --git a/src/infrastructure/cluster/PhabricatorDatabaseRef.php b/src/infrastructure/cluster/PhabricatorDatabaseRef.php --- a/src/infrastructure/cluster/PhabricatorDatabaseRef.php +++ b/src/infrastructure/cluster/PhabricatorDatabaseRef.php @@ -557,8 +557,14 @@ $conn = $this->newManagementConnection(); try { - $value = queryfx_one($conn, 'SELECT @@%Q', $key); - $value = $value['@@'.$key]; + $value = queryfx_one($conn, 'SELECT @@%C', $key); + + // NOTE: Although MySQL allows us to escape configuration values as if + // they are column names, the escaping is included in the column name + // of the return value: if we select "@@`x`", we get back a column named + // "@@`x`", not "@@x" as we might expect. + $value = head($value); + } catch (AphrontQueryException $ex) { $value = null; } diff --git a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php --- a/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php +++ b/src/infrastructure/edges/editor/PhabricatorEdgeEditor.php @@ -275,13 +275,13 @@ $conn_w->openTransaction(); $this->openTransactions[] = $conn_w; - foreach (array_chunk($sql, 256) as $chunk) { + foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) { queryfx( $conn_w, 'INSERT INTO %T (src, type, dst, dateCreated, seq, dataID) - VALUES %Q ON DUPLICATE KEY UPDATE dataID = VALUES(dataID)', + VALUES %LQ ON DUPLICATE KEY UPDATE dataID = VALUES(dataID)', PhabricatorEdgeConfig::TABLE_NAME_EDGE, - implode(', ', $chunk)); + $chunk); } } } @@ -320,9 +320,9 @@ foreach (array_chunk($sql, 256) as $chunk) { queryfx( $conn_w, - 'DELETE FROM %T WHERE (%Q)', + 'DELETE FROM %T WHERE %LO', PhabricatorEdgeConfig::TABLE_NAME_EDGE, - implode(' OR ', $chunk)); + $chunk); } } } diff --git a/src/infrastructure/edges/query/PhabricatorEdgeQuery.php b/src/infrastructure/edges/query/PhabricatorEdgeQuery.php --- a/src/infrastructure/edges/query/PhabricatorEdgeQuery.php +++ b/src/infrastructure/edges/query/PhabricatorEdgeQuery.php @@ -322,11 +322,11 @@ /** * @task internal */ - private function buildOrderClause($conn_r) { + private function buildOrderClause(AphrontDatabaseConnection $conn) { if ($this->order == self::ORDER_NEWEST_FIRST) { - return 'ORDER BY edge.dateCreated DESC, edge.seq DESC'; + return qsprintf($conn, 'ORDER BY edge.dateCreated DESC, edge.seq DESC'); } else { - return 'ORDER BY edge.dateCreated ASC, edge.seq ASC'; + return qsprintf($conn, 'ORDER BY edge.dateCreated ASC, edge.seq ASC'); } } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -123,12 +123,19 @@ AphrontDatabaseConnection $conn, $table_name) { + $table_alias = $this->getPrimaryTableAlias(); + if ($table_alias === null) { + $table_alias = qsprintf($conn, ''); + } else { + $table_alias = qsprintf($conn, '%T', $table_alias); + } + return qsprintf( $conn, '%Q FROM %T %Q %Q %Q %Q %Q %Q %Q', $this->buildSelectClause($conn), $table_name, - (string)$this->getPrimaryTableAlias(), + $table_alias, $this->buildJoinClause($conn), $this->buildWhereClause($conn), $this->buildGroupClause($conn), @@ -425,7 +432,7 @@ } else { // No paging is being applied to this query so we do not need to // construct a paging clause. - return ''; + return qsprintf($conn, ''); } $keys = array(); @@ -655,24 +662,16 @@ $conn, '%Q %Q %Q', $field, - $reverse ? '>' : '<', + $reverse ? qsprintf($conn, '>') : qsprintf($conn, '<'), $value); } if ($parts) { - if (count($parts) > 1) { - $clause[] = '('.implode(') OR (', $parts).')'; - } else { - $clause[] = head($parts); - } + $clause[] = qsprintf($conn, '%LO', $parts); } if ($clause) { - if (count($clause) > 1) { - $clauses[] = '('.implode(') AND (', $clause).')'; - } else { - $clauses[] = head($clause); - } + $clauses[] = qsprintf($conn, '%LA', $clause); } if ($value === null) { @@ -689,7 +688,11 @@ } } - return '('.implode(') OR (', $clauses).')'; + if ($clauses) { + return qsprintf($conn, '%LO', $clauses); + } + + return qsprintf($conn, ''); } @@ -1315,7 +1318,7 @@ return qsprintf( $conn, 'GROUP BY %Q', - $this->getApplicationSearchObjectPHIDColumn()); + $this->getApplicationSearchObjectPHIDColumn($conn)); } else { return qsprintf($conn, ''); } @@ -1339,16 +1342,16 @@ $alias = $constraint['alias']; $index = $constraint['index']; $cond = $constraint['cond']; - $phid_column = $this->getApplicationSearchObjectPHIDColumn(); + $phid_column = $this->getApplicationSearchObjectPHIDColumn($conn); switch ($cond) { case '=': // Figure out whether we need to do a LEFT JOIN or not. We need to // LEFT JOIN if we're going to select "IS NULL" rows. - $join_type = 'JOIN'; + $join_type = qsprintf($conn, 'JOIN'); foreach ($constraint['constraints'] as $query_constraint) { $op = $query_constraint->getOperator(); if ($op === PhabricatorQueryConstraint::OPERATOR_NULL) { - $join_type = 'LEFT JOIN'; + $join_type = qsprintf($conn, 'LEFT JOIN'); break; } } @@ -2437,9 +2440,9 @@ // this to a LEFT join. We'll use WHERE to select matching rows // later. if ($has_null) { - $join_type = 'LEFT'; + $join_type = qsprintf($conn, 'LEFT'); } else { - $join_type = ''; + $join_type = qsprintf($conn, ''); } $joins[] = qsprintf( @@ -2912,7 +2915,7 @@ if ($alias) { $col = qsprintf($conn, '%T.spacePHID', $alias); } else { - $col = 'spacePHID'; + $col = qsprintf($conn, 'spacePHID'); } if ($space_phids && $include_null) { diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -1149,11 +1149,10 @@ $map[$key] = qsprintf($conn, '%C = %ns', $key, $value); } } - $map = implode(', ', $map); $id = $this->getID(); $conn->query( - 'UPDATE %R SET %Q WHERE %C = '.(is_int($id) ? '%d' : '%s'), + 'UPDATE %R SET %LQ WHERE %C = '.(is_int($id) ? '%d' : '%s'), $this, $map, $this->getIDKeyForUse(), @@ -1255,11 +1254,24 @@ $parameter_exception); } } - $data = implode(', ', $data); + + switch ($mode) { + case 'INSERT': + $verb = qsprintf($conn, 'INSERT'); + break; + case 'REPLACE': + $verb = qsprintf($conn, 'REPLACE'); + break; + default: + throw new Exception( + pht( + 'Insert mode verb "%s" is not recognized, use INSERT or REPLACE.', + $mode)); + } $conn->query( - '%Q INTO %R (%LC) VALUES (%Q)', - $mode, + '%Q INTO %R (%LC) VALUES (%LQ)', + $verb, $this, $columns, $data);