Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Kamil Tekiela]

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

>

Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Guilliam Xavier]

On Fri, Aug 6, 2021 at 9:43 PM Kamil Tekiela  wrote:

> 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
$data` becoming `string $query` and the return type `string` becoming
`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

Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Kamil Tekiela]

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

Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Rowan Tommins]

On 06/08/2021 16:06, Larry Garfield wrote:

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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Larry Garfield]

On Thu, Aug 5, 2021, at 5:21 PM, Kamil Tekiela wrote:
> 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

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Kamil Tekiela]

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

[PHP-DEV] Re: [RFC] Add parse_query_string as an alternative to parse_str

[Tobias Nyholm]

Hey. 

I see the RFC [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 
.