Re: [RFC] [Discussion] Followup Improvements for ext/uri

Hi Ignace, Currently, in your proposal you have 2 Query objects. This will give the developper a lot of work to understand where, when and which object to choose and why. Is that complexity really needed? IMHO we may end up with a correct API ... that no-one will use. > > Just to reiterate what I wrote to Juris a few days ago: I'm open to unifying the two classes, but I'm just hesitant because of security and evolvability reasons (but the main one is security). With all that in mind I believe a single Uri\Query should be used. Its goal should be: > > - to be immutable > - to store the query in its decoded form. > - to manipulate and change the data in a consistent way. > > So far, I imagined the two QueryParams classes to be mutable because one of their main goals is to be able to build (~ mutate) query param list... But otherwise an immutable implementation would be useful for sure. Decoding/encoding should happen at the object boundaries but everything inside the object should > be done on decoded data. > > Yes, that's what I also had to find out based on my experience with implementing the POC, so I completely agree here. On a bonus side, it would be nice to have a mechanism in PHP that allows the application to switch > from the current parse_str usage to the new improved parsing > provided by the new class when > populating the _GET array. (So that deprecating > parse_str can be initiated in some distant future.) > This last observation/remark is not mandatory but nice to have. > > This is a very interesting remark, and I have not thought about this possibility yet. Generally, I agree with the idea, but my long-term goal (or wish) is to move away from using $_GET and $_POST to access request data in favor of using objects... So I most probably won't deal with trying to implement this idea. However, I'm willing to add a UriQueryParams::fromCurrentQueryString(), maybe even a UriQueryParams::fromCurrentBody() or similar factory methods if people like it. - in respect to parse_str, no mangled data should occur on parsing: > > Uh, I completely forgot about this behavior of parse_str(), and I definitely agree that mangling shouldn't happen. - Only accept scalar values, null, and array. If an object or a resource is detected a ValueError error > should be thrown. > > I wasn't sure what to do with objects, but I'm happy to skip their support, especially if they would cause issues. The rest of the suggestions align with my initial plans (maybe with the exception of throwing ValueError -- I wrote TypeError in the related section). - Remove the addition of indices if the array is a list. > > Yes, this also aligns with my initial plans. Best regards, Máté >