Hey.
I see the RFC [https://wiki.php.net/rfc/parse_str_alternative https://wiki.php.net/rfc/parse_str_alternative] is just a rename of parse_str()
I agree with this. parse_str() is a really confusing name and it does not behave as I expect it to. I very much support changing it to parse_query_string() and make sure it gets a return value.
// Tobias
PS. I hope this message will add as a thread on https://news-web.php.net/php.internals/115081 https://news-web.php.net/php.internals/115081.
Hi Internals,
I have added implementation for
https://wiki.php.net/rfc/parse_str_alternative. If there are no other
comments, I would like to start voting on Saturday.
Regards,
Kamil
I have added implementation for
https://wiki.php.net/rfc/parse_str_alternative. If there are no other
comments, I would like to start voting on Saturday.
I too would appreciate having a function in the PHP library that returns an array and that is named more intuitively than parse_str().
However, I would suggest naming it parse_query() instead of parse_query_string() as _string() is redundant and I see shorter function names being preferable when the intent of the function is clear.
I searched for prior art and it appears that Guzzle's PSR7 helper library v1.x had a namespaced parse_query() function:
https://github.com/guzzle/psr7/blob/1.x/src/functions.php#L299 https://github.com/guzzle/psr7/blob/1.x/src/functions.php#L299
I found Psr7\parse_query() being called on GitHub in 840 places using SourceGraph code search:
https://sourcegraph.com/search?q=context:global+lang:php+Psr7%5Cparse_query%28+count:all&patternType=literal https://sourcegraph.com/search?q=context:global+lang:php+Psr7%5Cparse_query%28+count:all&patternType=literal
There was only one (1) place with SourceGraph code search where someone named a function parse_query_string(), and that example did not use parse_query_string() in the same way as your RFC:
https://sourcegraph.com/github.com/k0a1a/hotglue2/-/blob/controller.inc.php?L344:1 https://sourcegraph.com/github.com/k0a1a/hotglue2/-/blob/controller.inc.php?L344:1
My takeaway is that PHP developers will easily be able to understand the intent if named parse_query(), especially those who might be familiar with it from Guzzle, and thus there is no need for the redundant _string().
And, of course, a parse_query() in the global namespace won't conflict with Guzzle's use because their function is namespaced with Guzzle\Psr7.
I feel that we are missing a chance to also improve how parse_str
algorithm is currently used, we could or should (?) use this opportunity
to fix some long shortcomings around parse_str.
Also, +1 to this.
-Mike
P.S. WordPress has a parse_query() method on their WP_Query() class, but as someone who has worked with WordPress for 10+ years even I don't see these two in conflict. One is a method scoped to the solution domain of its class and the other would be a function in PHP's global namespace.
I too would appreciate having a function in the PHP library that returns an array and that is named more intuitively than
parse_str().However, I would suggest naming it
parse_query()instead ofparse_query_string()as_string()is redundant and I see shorter function names being preferable when the intent of the function is clear.
I agree about the _string suffix removal. However, I know we have
parse_url() already, but parse_query() might be too generic. I would
suggest adding "http" to the name. And as we already have
http_build_query() I would rather see http_parse_query().
The RFC should target 8.2.
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
I agree about the _string suffix removal. However, I know we have
parse_url()already, but parse_query() might be too generic. I would
suggest adding "http" to the name. And as we already have
http_build_query()I would rather see http_parse_query().
+1. http_parse_query() is even better.
-Mike
http_parse_query() would get my vote (if I had vote karma :P)
I agree about the _string suffix removal. However, I know we have
parse_url()already, but parse_query() might be too generic. I would
suggest adding "http" to the name. And as we already have
http_build_query()I would rather see http_parse_query().+1. http_parse_query() is even better.
-Mike
--
To unsubscribe, visit: https://www.php.net/unsub.php
http_parse_query() would get my vote (if I had vote karma :P)
You have a php.net account[1], so you are supposed to have voting karma. :)
[1] http://people.php.net/sarciszewski
--
Christoph M. Becker
I agree about the _string suffix removal. However, I know we have
parse_url()already, but parse_query() might be too generic. I would
suggest adding "http" to the name. And as we already have
http_build_query()I would rather see http_parse_query().
+1 for http_parse_query() as it sounds a lot more natural and consistent.
parse_str assumes that the query separator is always a "&" which reduces
its usage to only parsing query using that particular character. Again this
might be seen as an edge case but no RFC prevents using any other
character. If you where to use another character you are bound to use some
userland code workaround to then feed the "normalized" string to parse_str
Both http_build_query and parse_str respect the
arg_separator.input INI
setting.
If we were to go for a http_parse_query, I think it makes sense to
support the same behavior and parameters.
Thank you,
Ayesh.
Hi Internals,
I have added implementation for
https://wiki.php.net/rfc/parse_str_alternative. If there are no other
comments, I would like to start voting on Saturday.Regards,
Kamil
I will be voting No on this as is.
Not because I'm against improving the query parsing tools; I'm in favor of that. But as noted, this RFC doesn't do enough to improve it. Returning a value and changing the name isn't enough to justify another global method. As others noted, the parsing rules themselves are fugly and need to be improved.
More importantly to me, though, it is the year of our lord 2021, and PHP APIs have no business pretending that arrays with possibly-missing values are a data structure. They're a satire of a data structure.
If you're parsing a string with known structure into something, that something should be a properly defined object. The new PhpToken is a good example of that shift done well. Don't give me an array where I have to mess around with isset() and crap like that.
Give me a properly defined HttpQuery object with named, type-enforced properties and meaningful methods. Give it a parse(string) static method and a __toString() method to convert back to a query string. Anything less is, frankly, not worth the effort.
--Larry Garfield
Give me a properly defined HttpQuery object with named, type-enforced properties and meaningful methods. Give it a parse(string) static method and a __toString() method to convert back to a query string. Anything less is, frankly, not worth the effort.
While I understand your general principle, I'm not sure what such an
object would look like. There is no "standard list" of query string keys
which could be named as properties on a built-in HttpQuery object; every
request is different, so the user would have to provide the object to be
"hydrated" in some form.
Maybe it would have to be a trait, so you could write something like this?
class GetUserQueryParams {
use HttpQueryParser;
private ?int $id;
private ?string $name;
}
$params = GetUserQueryParam::fromQueryString('?id=42');
Or a utility method that took a class name, and converted the query
string parameters to named constructor arguments?
class GetUserQueryParams {
public function __construct(
private ?int $id,
private ?string $name
) {}
}
$params = parse_query_string('?id=42', GetUserQueryParams::class);
Once you've started down that route, people will want a way to alias
property names (e.g. with attributes), have parallel handling for other
formats like JSON ... and before you know it you have the kind of
complexity which is much easier to manage as a userland library than
something tied to core.
On the other hand, if core provides a sane implementation of the parsing
into a generic "bag of key-value pairs" (which PHP calls an "array"),
that would provide a very useful building block for any userland library
to build on.
Regards,
--
Rowan Tommins
[IMSoP]
Hi Internals,
Thanks for all the feedback. I have changed the name to http_parse_query as
it looks like more people prefer that name. I have also updated
https://wiki.php.net/rfc/parse_str_alternative for 8.2 (sorry for the
confusion) and I added open points.
I hear two concerns currently about this RFC. Please note that my intention
was to provide a simple drop-in replacement. However, I agree that there is
a lot of room for improvement in this area, so I highly appreciate your
comments on how we can best tackle these open points.
I will not open this RFC for voting any time soon, as I want to let the
conversation run through to see what other options/concerns people have.
Please feel free to share your comments on what you think is the right path
to improve this functionality in PHP.
Regards,
Kamil
Hi Internals,
Thanks for all the feedback. I have changed the name to http_parse_query as
it looks like more people prefer that name. I have also updated
https://wiki.php.net/rfc/parse_str_alternative for 8.2 (sorry for the
confusion) and I added open points.
I hear two concerns currently about this RFC. Please note that my intention
was to provide a simple drop-in replacement. However, I agree that there is
a lot of room for improvement in this area, so I highly appreciate your
comments on how we can best tackle these open points.
I will not open this RFC for voting any time soon, as I want to let the
conversation run through to see what other options/concerns people have.
Please feel free to share your comments on what you think is the right path
to improve this functionality in PHP.Regards,
Kamil
Hi, thanks for the RFC.
I agree with Rowan in https://externals.io/message/115642 that
http_parse_query() should "mirror http_build_query() as closely as
possible", notably:
- have a similar signature (except for the first parameter
array|object $databecomingstring $queryand the return typestringbecoming
array, of course); - do not do name mangling (and for your question "what should happen to
mismatched square brackets?": not sure without an example, but I would say
"just keep them as is").
Best regards,
--
Guilliam Xavier
Hi Internals,
During my research into this topic, I discovered that there exists a
multibyte variant of this function in mbstring extension. This raises the
question: should we add a corresponding multibyte variant of
http_parse_query() to mbstring? Is there any usage in the wild of
mb_parse_str()?
I have even asked a question on Stack Overflow about this topic
https://stackoverflow.com/questions/68761695/can-mb-parse-str-produce-different-results-than-parse-str
To me, it doesn't look like parse_str() is useful anymore, but I come from
Europe where things happen mostly using a single encoding. I am not aware
of any problems that parse_str() might have when it comes to multiple
encoding types.
In fact, when looking at the whole input/output encoding feature of
mbstring I am getting a feeling that this is some niche feature that almost
nobody ever uses. While mbstring is useful for dealing with different
encodings, I am not sure if it is common for HTTP requests to be encoded
with anything else than UTF-8.
I would appreciate some input from experts who know more about mbstring and
encodings.
Regards,
Kamil