On Mon, Nov 25, 2019 at 10:30:55AM +0100, Christopher Faulet wrote:
> First, a key without value is not properly handled. You must not try to
> parse the value, otherwise the following parameter is read as value. For
> instance "/metrics?no-maint&scope=server".

oh good catch fixed.

> Then, if empty keys are forbidden (and it seems to be reasonable), you
> should throw an error if the delimiter is an equal sign (so a value with an
> empty key),  or, at least, you should skip the value. For instance,
> "/metrics?=value" or "/metrics?k1=v1&=v2". Another way to catch this case is
> to consider a key without value as equivalent to a value with an empty key.
> This way "/metrics?no-maint" could be equivalent to "/metrics?=no-maint".
> Your choice :) But be careful to make a difference between a key without
> value ("?a") and a key with an empty value ("?a=").

indeed, I made the mistake to continue when key was empty, without
ignoring the following value. I now ignore the value which follows.
in fact it now makes the code simpler as it removes a condition.
if you prefer to throw an error, let me know.

> The equal sign should probably be forbidden in a value (before decoding).
> For instance, "/metrics?k1=val=ue" or "/metrics?k1==v1".

fixed, an error is thrown is that case.

> Finally, it could be good to stop the parsing on the number sign (#). To not
> parse the fragment part of the uri, if any. The current version is also
> affected by this issue.

ok fixed.

You should receive v2 shortly.


Reply via email to