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.
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
---------------------------------------------------------------------
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 16:39:11 CET