On 01.02.2020 14:23, Stefan Sperling wrote: > On Sat, Feb 01, 2020 at 12:31:16PM +0100, Branko Čibej wrote: >> On 01.02.2020 12:17, Stefan Sperling wrote: >>> On Fri, Jan 31, 2020 at 04:12:20AM +0000, Daniel Shahaf wrote: >>>> 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? >>> This idea won't work without some restructuring because the redirect >>> retry loop is currently in libsvn_client, not within ra_serf. >> And for good reason. The RA layer is supposed do be stupid^Wsimple. >> >> -- Brane > Here is a patch I am running tests on now. With this, the RA layer returns > the redirect URL in both non-canonicalized and canonicalized forms. > The idea is to allow libsvn_client to cache the non-canonicalized redirect > URL and detect loops based on it, while using the canonicalized version for > any other purposes.
This seems like a good fix, given that we can't just return a non-canonical URL from the RA layer, due to API compatibility constraints. > I would not propose to backport this. Rather, this patch would go to 1.14 > only. Ack. Note that there's redirect-loop detection in JavaHL, too, separate from libsvn_client. It should receive the same kind of polishing. -- Brane
