wpf
wpf copied to clipboard
dotnet
BindUriHelper.UriToString is technically broken as it fails to truncate the uri
Description
UriToString is technically broken (both in .NET Framework and .NET Core) as it is supposed to truncate for lengths over 2083 chars but it fails to do so due to a misunderstanding how StringBuilder's capacity works from the original developer.
https://github.com/dotnet/wpf/blob/bdec3d515f52c55bc3a6431e71db97c88f868215/src/Microsoft.DotNet.Wpf/src/Shared/MS/Utility/BindUriHelper.cs#L67-L71
It is the same in .NET Framework: https://referencesource.microsoft.com/#PresentationFramework/src/Shared/MS/Utility/BindUriHelper.cs,60
Expected behavior
The URI is trimmed on string conversion if it exceeds MAX_URL_LENGTH.
Actual behavior
Unnecessary allocations are made and the string is not trimmed as intended.
Next steps
- We should definitely fix the unnecessary allocations.
- However, should we change it to the intended behavior, or should we keep the current one?
- or should we keep the current one?
At least for non-framework I hope not. If no one has ever complained about this then it's likely something nobody has noticed at all. Which wouldn't be a surprise as it's a very niche scenario. I think carrying bugs around only makes sense if fixing it can break many apps but if fixing it only breaks one app out of hundred thousand it's not worth to keep the bug.
On the contrary, if nobody every complained then I don't see why the current behavior should be changed.
I don't mind updating the code so that it does not pretend to be doing something it doesn't do, but starting to trim things that we never trimmed doesn't sound justified. If a component down the line doesn't support longer URIs, it should deal with it itself.
I don't see why the current behavior should be changed.
Because its a bug. That is a +1 for the why. Makes it 1:0 for making the change. If we keep this bug then we just have more and more .NET versions with it. The earlier it's getting fixed the less risk a developer runs into it.
How can a developer run into this?
What I am suggesting is to make it not a bug. Remove the argument and update the comment. Then there is no bug. And no documented behavior changes.
My own inclination is to keep the behaviour to be honest.
Besides major browsers that love to limit URIs to 2083; no real webservers have an actual issue with that, so in an app-world, this is perfectly acceptable. Same for pack URIs, even if you've created a local monstrosity, it shall not pose an issue.
The only problem is with WebBrowser / IWebBrowser2, which I guess is gonna cry out when we feed him longer URI. I'd need to test whether we can do that with those APIs or not and how they behave.
Agreed. And in HTTP context, I believe RFC 9110 now recommends supporting at least 8000 octets. Either way it's browser's responsibility to trim or error the URI if it doesn't like it.