[Moved from users@]
On 06.11.2018 19:16, Branko Čibej wrote:
> On 04.11.2018 20:11, Branko Čibej wrote:
>> On 04.11.2018 18:57, Thorsten Schöning wrote:
>>> Guten Tag Branko Čibej,
>>> am Sonntag, 4. November 2018 um 17:47 schrieben Sie:
>>>
>>>> I'm not sure what you mean by "handles more than only DAV successfully"
>>> I thought it might be possible that GitHub answers differently but
>>> properly, because the other check mentioned something about HTTP v2.
>>> Because of TLS, I was unable to look at the requests and responses
>>> then, but it's like you said, they don't provide DAV-headers in their
>>> response to OPTION.
>>>
>>>> And yes, the HTTP/DAV specification requires that header to be present
>>>> in the response.
>>> Which you didn't care about before and things worked for some years
>>> for some users.
>> We made this change because users complained about unhelpful error
>> messages when they tried to connect to a server that did not even
>> implement HTTP/DAV. The error message was "Malformed XML in response"
>> which wasn't exactly helpful for diagnosing the problem.
>>
>> I admit I didn't have GitHub in mind when I added this check. ...
> I added a test case to our suite that tries the following command:
>
> svn info https://github.com/apache/subversion/trunk
>
>
> It runs on one of our build slaves, so we'll know fairly soon when (if)
> GitHub deploys a fix. And, of course, we'll also know if this feature
> breaks again in future.
>
> http://svn.apache.org/r1845942
I created SVN-4789 to track this and added a patch with a possible fix.
I did not commit it because it contains what is, in my opinion, a really
horrible hack; but it does let our client work against
https://github.com again. I would very much prefer that GitHub adds that
trivial one-liner fix to their bridge implementation. However, if this
issue persists and prevents users from migrating to 1.11.x and later, we
may have to use this hack under protest. Note that it is in some ways
similar to the now-defunct ASP.NET hack.
[[[
Hack ra_serf to work with GitHub's bridge again.
* subversion/libsvn_ra_serf/options.c
(options_context_t): New member 'received_github_request_id_header'.
(capabilities_headers_iterator_callback): Check if we received that header.
(options_response_handler): Allow DAV responses without DAV headers, if
the 'X-GitHub-Request-Id' header is present.
* subversion/tests/cmdline/dav_tests.py
(connect_to_github_server):
Remove the @XFail annotation. Add an @Issue annotation.
Change the test URL to point at HTTPd's GitHub mirror.
Issue: SVN-4789
]]]
-- Brane
Index: subversion/libsvn_ra_serf/options.c
===================================================================
--- subversion/libsvn_ra_serf/options.c (revision 1846637)
+++ subversion/libsvn_ra_serf/options.c (working copy)
@@ -74,6 +74,10 @@
/* Have we received any DAV headers at all? */
svn_boolean_t received_dav_header;
+ /* ### FIXME: SVN-4789
+ Have we received the bespoke GitHub request ID header? */
+ svn_boolean_t received_github_request_id_header;
+
const char *activity_collection;
svn_revnum_t youngest_rev;
@@ -355,6 +359,22 @@
}
}
+ /* ### FIXME: SVN-4789
+ GitHub's Subversion bridge does not set any DAV: headers in its
+ response, even though it supports bits of our HTTP protocol and
+ returns a valid response body to an OPTIONS request. We try to work
+ around that monumental brokenness by checking for this
+ GitHub-specific response header.
+
+ Note that the case-sensitive string comparison here is intentional;
+ we do *not* want to support any number of strange hacks, so if
+ GitHub changes the case of their header name, this becomes their
+ problem entirely. */
+ else if (strcmp(key, "X-GitHub-Request-Id") == 0)
+ {
+ opt_ctx->received_github_request_id_header = TRUE;
+ }
+
return 0;
}
@@ -401,18 +421,28 @@
serf_bucket_headers_do(hdrs, capabilities_headers_iterator_callback,
opt_ctx);
- /* Bail out early if we're not talking to a DAV server.
- Note that this check is only valid if we've received a success
- response; redirects and errors don't count. */
- if (opt_ctx->handler->sline.code >= 200
- && opt_ctx->handler->sline.code < 300
- && !opt_ctx->received_dav_header)
+ /* ### FIXME: SVN-4789 */
+ if (!opt_ctx->received_github_request_id_header)
{
- return svn_error_createf
- (SVN_ERR_RA_DAV_OPTIONS_REQ_FAILED, NULL,
- _("The server at '%s' does not support the HTTP/DAV protocol"),
- session->session_url_str);
+ /* Bail out early if we're not talking to a DAV server. Note
+ that this check is only valid if we've received a success
+ response; redirects and errors don't count. */
+ if (opt_ctx->handler->sline.code >= 200
+ && opt_ctx->handler->sline.code < 300
+ && !opt_ctx->received_dav_header)
+ {
+ return svn_error_createf
+ (SVN_ERR_RA_DAV_OPTIONS_REQ_FAILED, NULL,
+ _("The server at '%s' does not support the WebDAV protocol"),
+ session->session_url_str);
+ }
}
+ else if (!opt_ctx->received_dav_header)
+ {
+ /* ### FIXME: SVN-4789
+ We really should emit a warning here, there's no API for that. */
+ opt_ctx->received_dav_header = TRUE;
+ }
/* Assume mergeinfo capability unsupported, if didn't receive information
about server or repository mergeinfo capability. */
Index: subversion/tests/cmdline/dav_tests.py
===================================================================
--- subversion/tests/cmdline/dav_tests.py (revision 1846637)
+++ subversion/tests/cmdline/dav_tests.py (working copy)
@@ -69,12 +69,15 @@
#----------------------------------------------------------------------
-@XFail()
+@Issue(4789)
@SkipUnless(svntest.main.is_remote_http_connection_allowed)
def connect_to_github_server(sbox):
"connect to GitHub's SVN bridge"
- github_mirror_url = 'https://github.com/apache/subversion/trunk'
+ # FIXME: For some reason, trying this against Subversion's own mirror
+ # always results in: 504 Gateway Timeout.
+ #github_mirror_url = 'https://github.com/apache/subversion/trunk/'
+ github_mirror_url = 'https://github.com/apache/httpd/trunk/'
# Skip this test if we can't connect to the GitHub server.
# We check this here instead of in a SkipUnless() predicate decorator,
