Stefan Sperling wrote:
On Thu, Jan 30, 2020 at 07:43:00PM +0000, Julian Foad wrote:
Meh... What a mess.
One part of the clean up: we should make very clear where a URL variable
holds a 'canonical' URL (in this conversation, that means according to svn's
own cannonicalisation rules) and where it does not. As our general rule is
URLs in Subversion source code should be canonical, the way to make this
clear is to label any non-canonical URL as such, by it's variable name, if it
exists for more than about one line of code from where it's received to where
it's validated/canonicalized
(I pressed 'Send' too early here, but was not going to add anything
significant.)
I don't think that will help much in this case. ra_serf is supposed to
return a "corrected URL" the client can use, instead of the orignal URL
which was already canonical. By definition, the output of this operation
must be canocalized if the "corrected URL" is going to be used anywhere.
Checking the code... OK, I see: what I'm suggesting is exactly what I
failed to do in my previous commit. I somehow thought it was normal in
this code area to return a non-canonical URL, but it isn't.
I'm not sure what is the best thing to do. Maybe the original "redirect
loop" failure mode, which I previously regarded as a bug, really is the
most appropriate thing, or at least good enough.
- Julian
