Julian Foad wrote on Thu, 30 Jan 2020 20:05 +0000:
> Stefan Sperling wrote:
> > 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.
That case is an impedance mismatch between our internal conventions and
the HTTP protocol.
In HTTP, I believe trailing slashes are significant: http://archive.apache.org/dist
and http://archive.apache.org/dist/ could be different resources.
Therefore, the redirect cannot be assumed to be a bug in the server's
configuration.
However, I think it's reasonable for a caller of svn_ra_open4() to
assume that if *CORRECTED_URL is set at all, then it is set to a
canonical URL that is different from REPOS_URL [the input URL].
So in balance, how about —
- ra_serf should not canonicalize redirection URLs before accessing them.
- After following all redirections (that is, once we get a 2xx response
or a 5xx response), canonicalize the resultant URL. If it is then
equal to the input URL, error out with a "Not a repository" error
[SVN_ERR_RA_SVN_REPOS_NOT_FOUND seems appropriate]; otherwise, return
SVN_ERR_RA_SESSION_URL_MISMATCH (the error code that svn_ra_open4()
promises for this situation).
This would fix both of the original bugs, wouldn't it?
Cheers,
Daniel
Received on 2020-01-31 05:12:38 CET