Karl Fogel wrote on Tue, 25 Mar 2008 at 11:38 -0400:
> Daniel Shahaf <d.s_at_daniel.shahaf.co.il> writes:
>> --- subversion/libsvn_ra/ra_loader.c (revision 30027)
>> +++ subversion/libsvn_ra/ra_loader.c (working copy)
>> @@ -468,10 +469,37 @@ svn_ra_create_callbacks(svn_ra_callbacks2_t **call
>> SVN_ERR(vtable->open_session(session, repos_URL, callbacks, callback_baton,
>> config, pool));
>>
>> + /* Check the UUID. */
>> + if (uuid)
>> + {
>> + const char *actual_uuid;
>> +
>> + SVN_ERR(vtable->get_uuid(session, &actual_uuid, pool));
>> +
>> + if (strcmp(uuid, actual_uuid) != 0)
>> + {
>> + return svn_error_createf(SVN_ERR_RA_UUID_MISMATCH, NULL,
>> + _("Repository UUID '%s' doesn't match "
>> + "expected UUID '%s'"),
>> + actual_uuid, uuid);
>> + }
>> + }
>> +
>> *session_p = session;
>> return SVN_NO_ERROR;
>> }
>
> Nice patch! Log message looks good, doc strings look good, etc.
>
> I'm worried about the above part of the change, though. Over http:// it
> adds an extra round-trip to the server.
>
It does add a round trip over neon, but I suspect it shouldn't.
Specifically, the following PROPFIND request, which neon makes early in
an 'svn up' connection:
<?xml version="1.0" encoding="utf-8"?>
<propfind xmlns="DAV:">
<prop>
<version-controlled-configuration xmlns="DAV:"/>
<resourcetype xmlns="DAV:"/>
<baseline-relative-path xmlns="http://subversion.tigris.org/xmlns/dav/"/>
<repository-uuid xmlns="http://subversion.tigris.org/xmlns/dav/"/>
</prop></propfind>
is sent twice if the patch is applied, and once if it isn't.
> Well, over serf it always adds an extra round trip, and I'm pretty sure
> that over neon it does as well, although in theory Neon caches the UUID
> after the first retrieval (and serf could be made to do same). But in
> any case, most of the time it probably won't be cached, because our RA
> session is new, so in practice there will be a new round trip.
>
> A better way might be to modify the server to always send back the UUID
> along with whatever information it was already sending back, and modify
> the client to grab that UUID if available and check that it matches.
> Obviously, if either client and server are old, then there will be no
> check, but that's okay -- we've gone this long without it, so a slow and
> soft upgrade cycle is acceptable.
>
> I realize that's probably a harder patch, but I'm really loathe to stack
> on more and more round trips. Our DAV layer already has too many.
>
> Thoughts?
>
> -Karl
>
Should this patch go into 1.5.0? I won't have time to set up an Apache
build environment in time for the release.
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-25 19:39:26 CET