Page MenuHomePhabricator
Files F15416301

D8315.diff
No OneTemporary
Actions

D8315.diff
View Options

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -23,6 +23,7 @@
'AphrontMySQLDatabaseConnectionBase' => 'aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php',
'AphrontMySQLiDatabaseConnection' => 'aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php',
'AphrontQueryAccessDeniedException' => 'aphront/storage/exception/AphrontQueryAccessDeniedException.php',
+ 'AphrontQueryCharacterSetException' => 'aphront/storage/exception/AphrontQueryCharacterSetException.php',
'AphrontQueryConnectionException' => 'aphront/storage/exception/AphrontQueryConnectionException.php',
'AphrontQueryConnectionLostException' => 'aphront/storage/exception/AphrontQueryConnectionLostException.php',
'AphrontQueryCountException' => 'aphront/storage/exception/AphrontQueryCountException.php',
@@ -442,6 +443,7 @@
'AphrontMySQLDatabaseConnectionBase' => 'AphrontDatabaseConnection',
'AphrontMySQLiDatabaseConnection' => 'AphrontMySQLDatabaseConnectionBase',
'AphrontQueryAccessDeniedException' => 'AphrontQueryRecoverableException',
+ 'AphrontQueryCharacterSetException' => 'AphrontQueryException',
'AphrontQueryConnectionException' => 'AphrontQueryException',
'AphrontQueryConnectionLostException' => 'AphrontQueryRecoverableException',
'AphrontQueryCountException' => 'AphrontQueryException',
Index: src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
===================================================================
--- src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
+++ src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
@@ -26,10 +26,14 @@
return;
}
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
return '<S>';
}
+ public function escapeBinaryString($string) {
+ return '<B>';
+ }
+
public function escapeColumnName($name) {
return '<C>';
}
Index: src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
===================================================================
--- src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
+++ src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
@@ -6,7 +6,12 @@
final class AphrontMySQLDatabaseConnection
extends AphrontMySQLDatabaseConnectionBase {
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
+ $this->validateUTF8String($string);
+ return $this->escapeBinaryString($string);
+ }
+
+ public function escapeBinaryString($string) {
return mysql_real_escape_string($string, $this->requireConnection());
}
Index: src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
===================================================================
--- src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
+++ src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
@@ -74,7 +74,7 @@
public function escapeStringForLikeClause($value) {
$value = addcslashes($value, '\%_');
- $value = $this->escapeString($value);
+ $value = $this->escapeUTF8String($value);
return $value;
}
@@ -321,4 +321,22 @@
return $this;
}
+ /**
+ * Check inserts for characters outside of the BMP. Even with the strictest
+ * settings, MySQL will silently truncate data when it encounters these, which
+ * can lead to data loss and security problems.
+ */
+ protected function validateUTF8String($string) {
+ if (phutil_is_utf8_with_only_bmp_characters($string)) {
+ return;
+ }
+
+ throw new AphrontQueryCharacterSetException(
+ pht(
+ 'Attempting to construct a query containing characters outside of '.
+ 'the Unicode Basic Multilingual Plane. MySQL will silently truncate '.
+ 'this data if it is inserted into a `utf8` column. Use the `%%B` '.
+ 'conversion to escape binary strings data.'));
+ }
+
}
Index: src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
===================================================================
--- src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
+++ src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
@@ -8,7 +8,12 @@
final class AphrontMySQLiDatabaseConnection
extends AphrontMySQLDatabaseConnectionBase {
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
+ $this->validateUTF8String($string);
+ return $this->escapeBinaryString($string);
+ }
+
+ public function escapeBinaryString($string) {
return $this->requireConnection()->escape_string($string);
}
Index: src/aphront/storage/exception/AphrontQueryCharacterSetException.php
===================================================================
--- /dev/null
+++ src/aphront/storage/exception/AphrontQueryCharacterSetException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class AphrontQueryCharacterSetException extends AphrontQueryException {
+
+}
Index: src/xsprintf/PhutilQsprintfInterface.php
===================================================================
--- src/xsprintf/PhutilQsprintfInterface.php
+++ src/xsprintf/PhutilQsprintfInterface.php
@@ -4,7 +4,8 @@
* @group storage
*/
interface PhutilQsprintfInterface {
- public function escapeString($string);
+ public function escapeBinaryString($string);
+ public function escapeUTF8String($string);
public function escapeColumnName($string);
public function escapeMultilineComment($string);
public function escapeStringForLikeClause($string);
Index: src/xsprintf/qsprintf.php
===================================================================
--- src/xsprintf/qsprintf.php
+++ src/xsprintf/qsprintf.php
@@ -5,8 +5,8 @@
* all the normal conversions (like %s) will be properly escaped, and
* additional conversions are supported:
*
- * %nd, %ns, %nf
- * "Nullable" versions of %d, %s and %f. Will produce 'NULL' if the
+ * %nd, %ns, %nf, %nB
+ * "Nullable" versions of %d, %s, %f and %B. Will produce 'NULL' if the
* argument is a strict null.
*
* %=d, %=s, %=f
@@ -16,12 +16,16 @@
*
* qsprintf($escaper, 'WHERE hatID %=d', $hat);
*
- * %Ld, %Ls, %Lf
- * "List" versions of %d, %s and %f. These are appropriate for use in
+ * %Ld, %Ls, %Lf, %LB
+ * "List" versions of %d, %s, %f and %B. These are appropriate for use in
* an "IN" clause. For example:
*
* qsprintf($escaper, 'WHERE hatID IN(%Ld)', $list_of_hats);
*
+ * %B ("Binary String")
+ * Escapes a string for insertion into a pure binary column, ignoring
+ * tests for characters outside of the basic multilingual plane.
+ *
* %T ("Table")
* Escapes a table name.
*
@@ -116,6 +120,7 @@
case 'd': // ...integer.
case 'f': // ...float.
case 's': // ...string.
+ case 'B': // ...binary string.
$pattern = substr_replace($pattern, '', $pos, 1);
$length = strlen($pattern);
$type = $next;
@@ -139,7 +144,13 @@
break;
case 's': // ...strings.
foreach ($value as $k => $v) {
- $value[$k] = "'".$escaper->escapeString($v)."'";
+ $value[$k] = "'".$escaper->escapeUTF8String((string)$v)."'";
+ }
+ $value = implode(', ', $value);
+ break;
+ case 'B': // ...binary strings.
+ foreach ($value as $k => $v) {
+ $value[$k] = "'".$escaper->escapeBinaryString((string)$v)."'";
}
$value = implode(', ', $value);
break;
@@ -162,7 +173,16 @@
if ($nullable && $value === null) {
$value = 'NULL';
} else {
- $value = "'".$escaper->escapeString($value)."'";
+ $value = "'".$escaper->escapeUTF8String((string)$value)."'";
+ }
+ $type = 's';
+ break;
+
+ case 'B': // Binary String
+ if ($nullable && $value === null) {
+ $value = 'NULL';
+ } else {
+ $value = "'".$escaper->escapeBinaryString((string)$value)."'";
}
$type = 's';
break;
@@ -230,7 +250,7 @@
*/
function _qsprintf_check_type($value, $type, $query) {
switch ($type) {
- case 'Ld': case 'Ls': case 'LC': case 'LA': case 'LO':
+ case 'Ld': case 'Ls': case 'LC': case 'LB':
if (!is_array($value)) {
throw new AphrontQueryParameterException(
$query,
@@ -274,7 +294,7 @@
}
break;
- case 'Ls': case 's':
+ case 'Ls': case 's': case 'LB': case 'B':
case '~': case '>': case '<': case 'K':
if (!is_null($value) && !is_scalar($value)) {
throw new AphrontQueryParameterException(
@@ -283,15 +303,6 @@
}
break;
- case 'LA': case 'LO':
- if (!is_null($value) && !is_scalar($value) &&
- !(is_array($value) && !empty($value))) {
- throw new AphrontQueryParameterException(
- $query,
- "Expected a scalar or null or non-empty array for ".
- "%{$type} conversion.");
- }
- break;
default:
throw new Exception("Unknown conversion '{$type}'.");
}

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 10:26 AM (3 d, 7 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7661142
Default Alt Text
D8315.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility