Mark,
On 3/24/26 12:15 PM, Mark Thomas wrote:
On 24/03/2026 15:44, Christopher Schultz wrote:
Konstantin,
On 3/24/26 7:24 AM, Konstantin Kolinko wrote:
вт, 24 мар. 2026 г. в 12:13, Mark Thomas <[email protected]>:
<snip/>
You have convinced me.
I withdraw my objection to handling "&" as equivalent to "&" in
encodeURL() and encodeRedirectURL().
I see a problem.
It is easy to implement removeQueryParameters() to remove either
"&" or "&",
but a problem is with the subsequent addNonce() call.
Yup. I was thinking about that as I wrote my previous post.
If the original URL had more than one parameter
then you can detect whether it used "&" or "%amp;", and use the same.
But if it had a single parameter then there is no way to decide what
delimiter should be used.
If you decide to implement this feature, please make it configurable.
+1
Unfortunately, I *think* the performance characteristics will be
different depending upon what kind of delimiter is being used.
Searching for a single code point ('&') is faster than searching for a
substring ("&"). I would also imagine that searching for '&' is
faster than searching for "&" (char vs string). I would like to keep
this as fast as possible, so I suspect there will only be two options,
here: & or & and the search-and-replacement will use two different
mechanisms.
Before I commit to using separate code, I'll set up a proper
performance test to see what the effective difference is. I think that
a speedup of more than 5% is probably worth it, given how often these
calls are likely to be made.
Currently you are removing the query parameter and the re-adding it.
What percentage of those operations are effectively a NO-OP? Is it high
enough to detect the NO-OP cases and do nothing?
Good question.
The most common case is that the URL doesn't have any csrf parameter in
it at all. If that happens, then the search will repeatedly search for
'&', match nothing, and make no modifications to the url String value.
If a URL is being repeatedly sent through response.encodeURL() during
the same request (which is the only time I can think of where we'd
replace csrf=ABC with the exact same token) then I think that's an
application problem. This code protects against that, but it also is
able to take a URL with an "old" token and replace it with the
most-recent one.
So I don't think I need to change the code at all at this point in order
to improve performance for the no-op case.
I'm happy to hear a counter-argument. It would be trivial to check to
see if the current value equals the discovered value.... but then the
logic needs to change to avoid adding it a *second time*.
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
