[essentials] ensure WebUtils.ParseQueryString captures fragment parameters

[rdavisau]

Description of Change

Makes WebUtils.ParseQueryString extract parameters from both Uri.Query and Uri.Fragment, resolving a regression from net8p4 to net8p5.

Issues Fixed

Fixes #15661

[ghost]

Hey there @rdavisau! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[rdavisau]

@dotnet-policy-service agree

[jfversluis]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[jstedfast]

My understanding is that query parameters MUST come before the URI fragment (which can only be terminated by the end of the URI).

https://www.rfc-editor.org/rfc/rfc3986#section-3.5

Can you show me where my interpretation is wrong?

There's also this syntax: URI = scheme ":" hier-part [ "?" query ] [ "#" fragment ] which, again, indicates that the fragment is always last.

[jstedfast]

Before this gets merged, lets make sure that it is valid to have a query string located within the fragment of a URI. I don't believe that it is valid.

[rdavisau]

Maybe the confusion comes from the name of the method WebUtils.ParseQueryString?

Originally, (despite the name) it had the behaviour of "extract all parameters from the url" and handled parameters whether they were in query string and/or fragment. That's covered by tests for the class and is also mentioned as the expected behaviour here. The perf change made ParseQueryString only extract from the query string, which is consistent with the name of the method, but fails the fragment test and breaks existing users of WebAuthenticator that rely on it handling parameters contained in a fragment of the auth response. This PR aims to undo that regression specifically. The ParseQueryString method is not part of the public API so I think it could be ok to rename.

[jstedfast]

Well, based on the comment that specifically states that it is meant to parse the fragment as if it contained query parameters, then I guess the correct solution here is to parse both sets as your patch does.