On Fri, Nov 21, 2008 at 10:15 PM, Greg Stein <gstein_at_gmail.com> wrote:
> 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.
Heh, ok, I assumed it was legal because I saw all the other
un-enclosed URIs. Will fix. :-)
>
> 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
I wanted to do this as my first instinct, but the get_vsn_options()
function doesn't have the ability to create arbitrary response
headers. It returns header *values* to mod_dav, and mod_dav then
slaps a DAV: in front. Any solution here?
>>...
>> +++ 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.
Well, we can say that they represent revision resources all we want,
but we have *no* other use cases at the moment other than accessing
revprops. So the comment is simply stating how the stub is
immediately useful. Do you really expect that revision resources will
have other uses someday?
>
>>...
>> +++ 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.
Sounds good, but I'm not seeing the mechanics here either.
get_option() is called by mod_dav on a specific resource, and only
gives us a chance to append text to the OPTIONS body response.
There's no way to tweak the response headers from that routine, is
there?
---------------------------------------------------------------------
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 06:21:48 CET