[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r34322 - in branches/http-protocol-v2/subversion: include mod_dav_svn

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 21 Nov 2008 20:15:07 -0800

On Fri, Nov 21, 2008 at 13:58, <sussman_at_tigris.org> wrote:
>...
\> +++ branches/http-protocol-v2/subversion/include/svn_dav.h Fri
Nov 21 13:58:04 2008 (r34322)
> @@ -182,6 +182,42 @@ extern "C" {
> #define SVN_DAV_NS_DAV_SVN_PARTIAL_REPLAY\
> SVN_DAV_PROP_NS_DAV "svn/partial-replay"
>
> +/** Presence of this in a DAV header in an OPTIONS response indicates
> + * that the server speaks HTTP protocol v2. This header provides an
> + * opaque URI that the client should send all custom REPORT requests
> + * against. Note: The URI actually follows *after* this header value
> + * using an equals sign, e.g.
> + *
> + * DAV: http://subversion.tigris.org/xmlns/dav/svn/root-stub=!svn/me

Ugh. That is not legal.

http://www.webdav.org/specs/rfc4918.html#HEADER_DAV

If you're going to use a URI, then it must be enclosed in angle
brackets. <http://example.com/foo> ... if not a URI, then use a plain
old token. The above value is neither.

And yes, I see that svn already has broken values in the DAV: header.
Fine. Let's not make it suck more, and I'll ponder on a way to get rid
of that. But for now, let's not continue to make it worse.

Further, the "=" is not a valid separator. That is a perfectly normal
character in a URI, so you can't just arbitrarily separate the URI
from the relative-path using that.

Suggestion: since the DAV: header is mostly about advertising
features, and since these new bits are key/value pairs, then they
should be advertised as response headers.

SVN-Root-Stub: !svn/me

>...
> +++ branches/http-protocol-v2/subversion/mod_dav_svn/dav_svn.h Fri Nov 21 13:58:04 2008 (r34322)
> @@ -39,6 +39,18 @@ extern "C" {
> #endif /* __cplusplus */
>
>
> +/* HTTP Protocol version 2 -- with no DeltaV. We define these 'stubs'
> + as opaque fixtures to return to the client, and the client freely
> + appends paths to them. */
> +
> +#define DAV_SVN__ROOT_STUB "!svn/me" /* where REPORTS are sent */
> +#define DAV_SVN__PEGREV_STUB "!svn/bc" /* for accessing REV/PATH pairs */
> +#define DAV_SVN__REV_STUB "!svn/rev" /* for acccesing rev props */

No. Those are revision resources. The comment should state such.

And, gee, one of the operations on that revision is to get its
properties. But the stub is for constructing URLs to resources that
represent revisions.

>...
> +++ branches/http-protocol-v2/subversion/mod_dav_svn/version.c Fri Nov 21 13:58:04 2008 (r34322)
>...
> @@ -127,6 +127,8 @@ open_txn(svn_fs_txn_t **ptxn,
> + /* HTTP protocol v2: client may request some info specific
> + to the resource. */
> + if ((strcmp(elem->name, "youngest-rev") == 0)
> + && resource->info->repos
> + && resource->info->repos->fs)

I think this should be returned in the headers of the response to the
OPTIONS request (e.g. SVN-Youngest-Revision: 123). No conditionals.
Just return the value.

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-22 05:15:21 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.