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é

>

Reply via email to