Page MenuHomePhabricator
Differential D20136 Diff 48106 src/parser/PhutilURI.php

Changeset View

Standalone View

View Options

src/parser/PhutilURI.php

Show First 20 Lines • Show All 117 Lines • ▼ Show 20 Lines public function __construct($uri) {
$this->protocol = idx($parts, 'scheme', ''); $this->protocol = idx($parts, 'scheme', '');
$this->user = rawurldecode(idx($parts, 'user', '')); $this->user = rawurldecode(idx($parts, 'user', ''));
$this->pass = rawurldecode(idx($parts, 'pass', '')); $this->pass = rawurldecode(idx($parts, 'pass', ''));
$this->domain = idx($parts, 'host', ''); $this->domain = idx($parts, 'host', '');
$this->port = (string)idx($parts, 'port', ''); $this->port = (string)idx($parts, 'port', '');
$this->path = idx($parts, 'path', ''); $this->path = idx($parts, 'path', '');
$query = idx($parts, 'query'); $query = idx($parts, 'query');
if ($query) { if ($query) {
$this->query = id(new PhutilQueryStringParser())->parseQueryString( $pairs = id(new PhutilQueryStringParser())
$query); ->parseQueryStringToPairList($query);
foreach ($pairs as $pair) {
list($key, $value) = $pair;
$this->appendQueryParam($key, $value);
}
} }
$this->fragment = idx($parts, 'fragment', ''); $this->fragment = idx($parts, 'fragment', '');
$this->type = $type; $this->type = $type;
} }
public function __toString() { public function __toString() {
$prefix = null; $prefix = null;
Show All 33 Lines if (strlen($protocol) || strlen($auth) || strlen($domain)) {
} }
if (strlen($port)) { if (strlen($port)) {
$prefix .= ':'.$port; $prefix .= ':'.$port;
} }
} }
if ($this->query) { if ($this->query) {
$query = '?'.phutil_build_http_querystring($this->query); $query = '?'.phutil_build_http_querystring_from_pairs($this->query);
} else { } else {
$query = null; $query = null;
} }
if (strlen($this->getFragment())) { if (strlen($this->getFragment())) {
$fragment = '#'.$this->getFragment(); $fragment = '#'.$this->getFragment();
} else { } else {
$fragment = null; $fragment = null;
} }
$path = $this->getPath(); $path = $this->getPath();
if ($this->isGitURI()) { if ($this->isGitURI()) {
if (strlen($path)) { if (strlen($path)) {
$path = ':'.$path; $path = ':'.$path;
} }
} }
return $prefix.$path.$query.$fragment; return $prefix.$path.$query.$fragment;
} }
public function setQueryParam($key, $value) { public function setQueryParam($key, $value) {
if ($value === null) { // To set, we replace the first matching key with the new value, then
unset($this->query[$key]); // 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, '');
amckinleyUnsubmitted
Not Done
Inline Actions

Wait, what does list($key) do? I was expecting this to be a no-op, but it appears to just drop the 2nd element of the array:

$foo = array('bang', 'bar');
list($x) = $foo;

var_dump($x);
// string(4) "bang"

Any reason to prefer this construction over $key = head(phutil_http_parameter_pair($key, ''))?

amckinley: Wait, what does `list($key)` do? I was expecting this to be a no-op, but it appears to just…
epriestleyAuthorUnsubmitted
Done
Inline Actions

list($key) = $x is the same as $key = head($x), it's just for consistency with list($key, $value) = $x.

epriestley: `list($key) = $x` is the same as `$key = head($x)`, it's just for consistency with `list($key…
$found = false;
foreach ($this->query as $list_key => $pair) {
amckinleyUnsubmitted
Not Done
Inline Actions

$list_key is more like $idx, right? And should always be naturally-ordered keys?

amckinley: `$list_key` is more like `$idx`, right? And should always be naturally-ordered keys?
epriestleyAuthorUnsubmitted
Done
Inline Actions

Yeah.

epriestley: Yeah.
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 { } else {
$this->query[$key] = $value; $this->insertQueryParam($key, $value, $list_key);
}
} }
$this->query = array_values($this->query);
amckinleyUnsubmitted
Not Done
Inline Actions

And shouldn't this be redundant unless someone has screwed up and tried to invoke insertQueryParam("key", "value", new Banana())?

amckinley: And shouldn't this be redundant unless someone has screwed up and tried to invoke…
epriestleyAuthorUnsubmitted
Done
Inline Actions

This may have an effect if you have a list like this:

a=a
b=1
a=a
b=2
a=a

...then set(b, 3). It will replace b=1 and leave a hole where b=2 used to be.

epriestley: This may have an effect if you have a list like this: ``` a=a b=1 a=a b=2 a=a ``` ...then…
// 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);
amckinleyUnsubmitted
Not Done
Inline Actions

And if the above line isn't redundant, shouldn't we be doing it after appendQueryParam()?

amckinley: And if the above line isn't redundant, shouldn't we be doing it after `appendQueryParam()`?
epriestleyAuthorUnsubmitted
Done
Inline Actions

(It's redundant after appending.)

epriestley: (It's redundant after appending.)
}
}
return $this; return $this;
} }
public function setQueryParams(array $params) { public function setQueryParams(array $params) {
$this->query = $params; $this->query = array();
foreach ($params as $k => $v) {
$this->appendQueryParam($k, $v);
}
amckinleyUnsubmitted
Not Done
Inline Actions

Maybe throw if $this->query isn't still equal to array()? There's nothing wrong with implementing it like this, other than potentially surprising a caller who has a URL with existing embedded query params, and then a list of params they intend to overwrite. (Or add a second $replace_existing_params = true argument)?

amckinley: Maybe `throw` if `$this->query` isn't still equal to `array()`? There's nothing wrong with…
epriestleyAuthorUnsubmitted
Done
Inline Actions

This is mostly about preserving the existing behavior, since we have a fair number of callers assuming they can do stuff like setQueryParams(array()) to remove the query parameters.

I agree that set isn't a great name in the abstract, and doesn't make it obvious whether it means "clear, then append" or "overwrite keys without clearing".

epriestley: This is mostly about preserving the existing behavior, since we have a fair number of callers…
return $this; return $this;
} }
/**
* @deprecated
*/
public function getQueryParams() { 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; 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) { public function setProtocol($protocol) {
$this->protocol = $protocol; $this->protocol = $protocol;
return $this; return $this;
} }
public function getProtocol() { public function getProtocol() {
return $this->protocol; return $this->protocol;
} }
▲ Show 20 Lines • Show All 164 Lines • Show Last 20 Lines
Phabricator · Built by Phacility