Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13195491
D20136.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D20136.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -529,6 +529,7 @@
'phlog' => 'error/phlog.php',
'pht' => 'internationalization/pht.php',
'phutil_build_http_querystring' => 'utils/utils.php',
+ 'phutil_build_http_querystring_from_pairs' => 'utils/utils.php',
'phutil_censor_credentials' => 'utils/utils.php',
'phutil_console_confirm' => 'console/format.php',
'phutil_console_format' => 'console/format.php',
@@ -559,6 +560,7 @@
'phutil_get_signal_name' => 'future/exec/execx.php',
'phutil_get_system_locale' => 'utils/utf8.php',
'phutil_hashes_are_identical' => 'utils/utils.php',
+ 'phutil_http_parameter_pair' => 'utils/utils.php',
'phutil_implode_html' => 'markup/render.php',
'phutil_ini_decode' => 'utils/utils.php',
'phutil_is_hiphop_runtime' => 'utils/utils.php',
diff --git a/src/future/aws/PhutilAWSv4Signature.php b/src/future/aws/PhutilAWSv4Signature.php
--- a/src/future/aws/PhutilAWSv4Signature.php
+++ b/src/future/aws/PhutilAWSv4Signature.php
@@ -242,14 +242,10 @@
private function getCanonicalParameterList(HTTPSFuture $future) {
$uri = new PhutilURI($future->getURI());
- $params = $uri->getQueryParams();
+ $params = $uri->getQueryParamsAsMap();
ksort($params);
- $canonical_parameters = array();
- foreach ($params as $key => $value) {
- $canonical_parameters[] = rawurlencode($key).'='.rawurlencode($value);
- }
- $canonical_parameters = implode('&', $canonical_parameters);
+ $canonical_parameters = phutil_build_http_querystring($params);
return $canonical_parameters;
}
diff --git a/src/future/oauth/PhutilOAuth1Future.php b/src/future/oauth/PhutilOAuth1Future.php
--- a/src/future/oauth/PhutilOAuth1Future.php
+++ b/src/future/oauth/PhutilOAuth1Future.php
@@ -94,7 +94,7 @@
}
$params = $params
- + $this->uri->getQueryParams()
+ + $this->uri->getQueryParamsAsMap()
+ $this->getOAuth1Headers();
return $this->sign($params);
diff --git a/src/parser/PhutilURI.php b/src/parser/PhutilURI.php
--- a/src/parser/PhutilURI.php
+++ b/src/parser/PhutilURI.php
@@ -123,8 +123,13 @@
$this->path = idx($parts, 'path', '');
$query = idx($parts, 'query');
if ($query) {
- $this->query = id(new PhutilQueryStringParser())->parseQueryString(
- $query);
+ $pairs = id(new PhutilQueryStringParser())
+ ->parseQueryStringToPairList($query);
+
+ foreach ($pairs as $pair) {
+ list($key, $value) = $pair;
+ $this->appendQueryParam($key, $value);
+ }
}
$this->fragment = idx($parts, 'fragment', '');
@@ -174,7 +179,7 @@
}
if ($this->query) {
- $query = '?'.phutil_build_http_querystring($this->query);
+ $query = '?'.phutil_build_http_querystring_from_pairs($this->query);
} else {
$query = null;
}
@@ -196,23 +201,114 @@
}
public function setQueryParam($key, $value) {
- if ($value === null) {
- unset($this->query[$key]);
- } else {
- $this->query[$key] = $value;
+ // To set, we replace the first matching key with the new value, then
+ // remove all other matching keys. This replaces the old value and retains
+ // the parameter order.
+
+ $is_null = ($value === null);
+
+ // Typecheck and cast the key before we compare it to existing keys. This
+ // raises an early exception if the key has a bad type.
+ list($key) = phutil_http_parameter_pair($key, '');
+
+ $found = false;
+ foreach ($this->query as $list_key => $pair) {
+ list($k, $v) = $pair;
+
+ if ($k !== $key) {
+ continue;
+ }
+
+ if ($found) {
+ unset($this->query[$list_key]);
+ continue;
+ }
+
+ $found = true;
+
+ if ($is_null) {
+ unset($this->query[$list_key]);
+ } else {
+ $this->insertQueryParam($key, $value, $list_key);
+ }
+ }
+
+ $this->query = array_values($this->query);
+
+ // If we didn't find an existing place to put it, add it to the end.
+ if (!$found) {
+ if (!$is_null) {
+ $this->appendQueryParam($key, $value);
+ }
}
+
return $this;
}
public function setQueryParams(array $params) {
- $this->query = $params;
+ $this->query = array();
+
+ foreach ($params as $k => $v) {
+ $this->appendQueryParam($k, $v);
+ }
+
return $this;
}
+ /**
+ * @deprecated
+ */
public function getQueryParams() {
+ $map = array();
+
+ foreach ($this->query as $pair) {
+ list($k, $v) = $pair;
+ $map[$k] = $v;
+ }
+
+ return $map;
+ }
+
+ public function getQueryParamsAsMap() {
+ $map = array();
+
+ foreach ($this->query as $pair) {
+ list($k, $v) = $pair;
+
+ if (isset($map[$k])) {
+ throw new Exception(
+ pht(
+ 'Query parameters include a duplicate key ("%s") and can not be '.
+ 'nondestructively represented as a map.',
+ $k));
+ }
+
+ $map[$k] = $v;
+ }
+
+ return $map;
+ }
+
+ public function getQueryParamsAsPairList() {
return $this->query;
}
+ public function appendQueryParam($key, $value) {
+ return $this->insertQueryParam($key, $value);
+ }
+
+ private function insertQueryParam($key, $value, $idx = null) {
+ list($key, $value) = phutil_http_parameter_pair($key, $value);
+
+ if ($idx === null) {
+ $this->query[] = array($key, $value);
+ } else {
+ $this->query[$idx] = array($key, $value);
+ }
+
+ return $this;
+ }
+
public function setProtocol($protocol) {
$this->protocol = $protocol;
return $this;
diff --git a/src/parser/__tests__/PhutilURITestCase.php b/src/parser/__tests__/PhutilURITestCase.php
--- a/src/parser/__tests__/PhutilURITestCase.php
+++ b/src/parser/__tests__/PhutilURITestCase.php
@@ -16,9 +16,12 @@
$this->assertEqual('/path/', $uri->getPath(), pht('path'));
$this->assertEqual(
array(
- 'query' => 'value',
+ array(
+ 'query',
+ 'value',
+ ),
),
- $uri->getQueryParams(),
+ $uri->getQueryParamsAsPairList(),
'query params');
$this->assertEqual('fragment', $uri->getFragment(), pht('fragment'));
$this->assertEqual(
@@ -37,7 +40,7 @@
$this->assertEqual('/example/example.git', $uri->getPath(), pht('path'));
$this->assertEqual(
array(),
- $uri->getQueryParams(),
+ $uri->getQueryParamsAsPairList(),
pht('query parameters'));
$this->assertEqual('', $uri->getFragment(), pht('fragment'));
$this->assertEqual(
@@ -183,9 +186,12 @@
$this->assertEqual('', $uri->getPath(), pht('path'));
$this->assertEqual(
array(
- 'x' => '/',
+ array(
+ 'x',
+ '/',
+ ),
),
- $uri->getQueryParams());
+ $uri->getQueryParamsAsPairList());
// This is not a legitimate URI and should not parse as one.
$uri = new PhutilURI('fruit.list: apple banana cherry');
@@ -275,4 +281,111 @@
$this->assertTrue($caught instanceof Exception);
}
+ public function testDuplicateKeys() {
+ $uri = new PhutilURI('http://www.example.com/?x=1&x=2');
+ $this->assertEqual(
+ 'http://www.example.com/?x=1&x=2',
+ (string)$uri);
+
+ $uri->appendQueryParam('x', '3');
+ $this->assertEqual(
+ 'http://www.example.com/?x=1&x=2&x=3',
+ (string)$uri);
+
+ $uri->setQueryParam('x', '4');
+ $this->assertEqual(
+ 'http://www.example.com/?x=4',
+ (string)$uri);
+
+ $uri->setQueryParam('x', null);
+ $this->assertEqual(
+ 'http://www.example.com/',
+ (string)$uri);
+
+ $uri->appendQueryParam('a', 'a');
+ $uri->appendQueryParam('b', 'b');
+ $uri->appendQueryParam('c', 'c');
+ $uri->appendQueryParam('b', 'd');
+
+ $this->assertEqual(
+ 'http://www.example.com/?a=a&b=b&c=c&b=d',
+ (string)$uri);
+
+ $uri->setQueryParam('b', 'e');
+ $this->assertEqual(
+ 'http://www.example.com/?a=a&b=e&c=c',
+ (string)$uri,
+ pht(
+ 'Replacing a parameter should retain position and overwrite other '.
+ 'instances of the key.'));
+ }
+
+ public function testBadHTTPParameters() {
+ $uri = new PhutilURI('http://www.example.com/');
+
+ $caught = null;
+ try {
+ $uri->setQueryParam(array(), 'x');
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertTrue(
+ (bool)$caught,
+ pht('Nonscalar HTTP keys should throw.'));
+
+ $caught = null;
+ try {
+ $uri->setQueryParam('x', array());
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertTrue(
+ (bool)$caught,
+ pht('Nonscalar HTTP values should throw.'));
+ }
+
+ public function testHTTPParameterTypes() {
+ // Whether you pass an integer or string, "0" should always be the same
+ // query parameter.
+
+ $uri = new PhutilURI('http://www.example.com/');
+
+ $uri->appendQueryParam(0, 'a');
+ $uri->appendQueryParam('0', 'b');
+ $this->assertEqual(
+ 'http://www.example.com/?0=a&0=b',
+ (string)$uri);
+
+ $uri->setQueryParam(0, 'c');
+ $this->assertEqual(
+ 'http://www.example.com/?0=c',
+ (string)$uri);
+
+ $uri->setQueryParam(0, 'a');
+ $uri->appendQueryParam('0', 'b');
+ $this->assertEqual(
+ 'http://www.example.com/?0=a&0=b',
+ (string)$uri);
+
+ $uri->setQueryParam('0', 'c');
+ $this->assertEqual(
+ 'http://www.example.com/?0=c',
+ (string)$uri);
+ }
+
+ public function testGetQueryParamsAsMap() {
+ $uri = new PhutilURI('http://www.example.com/?x=1&x=2');
+
+ $caught = null;
+ try {
+ $map = $uri->getQueryParamsAsMap();
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertTrue((bool)$caught);
+ }
+
}
diff --git a/src/utils/utils.php b/src/utils/utils.php
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -1567,19 +1567,37 @@
* @return string HTTP query string.
*/
function phutil_build_http_querystring(array $parameters) {
+ $pairs = array();
+ foreach ($parameters as $key => $value) {
+ $pairs[] = array($key, $value);
+ }
+
+ return phutil_build_http_querystring_from_pairs($pairs);
+}
+
+/**
+ * Build a query string from a list of parameter pairs.
+ *
+ * @param list<pair<string, string>> List of pairs.
+ * @return string HTTP query string.
+ */
+function phutil_build_http_querystring_from_pairs(array $pairs) {
// We want to encode in RFC3986 mode, but "http_build_query()" did not get
// a flag for that mode until PHP 5.4.0. This is equivalent to calling
// "http_build_query()" with the "PHP_QUERY_RFC3986" flag.
$query = array();
- foreach ($parameters as $key => $value) {
- if (is_array($value) || is_object($value)) {
+ foreach ($pairs as $pair_key => $pair) {
+ if (!is_array($pair) || (count($pair) !== 2)) {
throw new Exception(
pht(
- 'HTTP query parameter (with key "%s") is not a scalar. Parameters '.
- 'must all be scalars.',
- $key));
+ 'HTTP parameter pair (with key "%s") is not valid: each pair must '.
+ 'be an array with exactly two elements.',
+ $pair_key));
}
+
+ list($key, $value) = $pair;
+ list($key, $value) = phutil_http_parameter_pair($key, $value);
$query[] = rawurlencode($key).'='.rawurlencode($value);
}
$query = implode($query, '&');
@@ -1587,6 +1605,41 @@
return $query;
}
+/**
+ * Typecheck and cast an HTTP key-value parameter pair.
+ *
+ * Scalar values are converted to strings. Nonscalar values raise exceptions.
+ *
+ * @param scalar HTTP parameter key.
+ * @param scalar HTTP parameter value.
+ * @return pair<string, string> Key and value as strings.
+ */
+function phutil_http_parameter_pair($key, $value) {
+ try {
+ assert_stringlike($key);
+ } catch (InvalidArgumentException $ex) {
+ throw new PhutilProxyException(
+ pht('HTTP query parameter key must be a scalar.'),
+ $ex);
+ }
+
+ $key = (string)$key;
+
+ try {
+ assert_stringlike($value);
+ } catch (InvalidArgumentException $ex) {
+ throw new PhutilProxyException(
+ pht(
+ 'HTTP query parameter value (for key "%s") must be a scalar.',
+ $key),
+ $ex);
+ }
+
+ $value = (string)$value;
+
+ return array($key, $value);
+}
+
function phutil_decode_mime_header($header) {
if (function_exists('iconv_mime_decode')) {
return iconv_mime_decode($header, 0, 'UTF-8');
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, May 13, 10:26 PM (4 w, 33 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6276580
Default Alt Text
D20136.diff (12 KB)
Attached To
Mode
D20136: Fix destructive representation of "?x=1&x=2" in PhutilURI so it can raise errors immediately
Attached
Detach File
Event Timeline
Log In to Comment