On Wed, Jul 15, 2026 at 10:34 PM Máté Kocsis <[email protected]> wrote:
> Hi Ignace, > > Thanks for your feedback! > > I'm fine with separating the options to their own object. However, what I > would like to make sure about first is whether > the limits should really be applicable for parsing methods only, or > rather other methods should also respect them? > > - $parsingMaxQueryStringLength: This option is only useful during > parsing, so it's out of scope for my question. > - $parsingMaxParamCount: the append() and set() modification methods > could also respect a $maxParamCount option, > triggering an exception when the query parameter list exceeds the max > parameter count. > - $parsingMaxNestingLevel: similarly, append() and set() could also > respect it (e.g. by counting the number > of opening brackets in the query param name (?)). Enforcing this option > during parsing and modification would result in > some performance penalty, since the param name doesn't need introspection > during these operations otherwise. > That's why an alternative is to make a $maxNestingLevel option exclusive > to the toArray() method, because that's the > only time nesting level actually comes into play (and the brackets need > parsing at that point anyway). The possible problem > with this delayed validation is that possibly harmful query strings can > be passed along more easily. > > The reason why I originally thought about supporting these options during > only parsing is because I'd think that > that's the most uncontrollable part of query parameter handling. E.g. one > can enforce a maximum query param count > easily after parsing if this is needed (e.g. checking count() before > calling append() or set()). > > ... the same for the QueryParamBuildingOptions but for the`to*` methods. > > > Actually, the building options should be passed to append() and set(). > It's because their $value parameter is type juggled to > string or null immediately, thus their original type gets lost afterwards. > > On one hand, this behavior is useful for the hasValue() or deleteValue() > methods which try to find their $value param in the > query param list (comparing the type juggled $value with the stored > values -> a boolean "true" $value will be equal to a string "1" > in the list without the need to type juggle all stored query params). > > On the other hand though, this behavior might lead to interoperability > issues, e.g. when two libraries use different building options > to write to the same QueryParams.... Although, I think this is also a > possible problem even if building options are not supported, > so probably it's not a blocking issue. But most probably, passing them > during construction would slightly alleviate this problem, > because all writes would use the exact same config. > > Regards, > Máté > Hi Màté, I believe the question is more about domain boundaries. AFAIK: - HTTP servers already have a cap on how long an URL and thus a query string can be - the query string nesting or parsing depends only on how it is being consumed by the client and should not be restricted by the URI producer. Caveats: - the supported URI length depends on the server, CLI scripts and user scripts are able to bypass those HTTP server restrictions. Here's my thoughts: - Parsing and building are at the boundary of the system and control the I/O flow - Parsing makes the system behave like a consumer, Building makes the system behave like a producer. - I consider that dynamic methods are part of the domain because they only deal with PHP data not with external data (we do not allow concatenating 2 query string for instance) So I would not add the parsing options to the class modifier because on itself PHP should not add more constraints than needed. business rules might but IMHO this is out of scope for a low level API. Does PHP work with more nested levels or query parameters, the answer is yes. Should we recommend doing so, hell no but this is a business constraint unless it is a clear and known security issue when building the query string. On the other hand, I do agree that being able to configure how boolean and null values are handled when using any modifier methods is a needed feature. TL;DR: Adding the `QueryParamBuildingOptions` options as the last argument with sensible default to any modifier methods seems reasonable to me but adding public int $maxParamCount = 1000, public int $maxNestingLevel = 64, to `QueryParamBuildingOptions` without a clear and realistic example where this is solving a security issue seems like too much of a restriction for the API. Best regards, Ignace
