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
