svn commit: r1655458 - in /subversion/trunk/subversion: libsvn_ra/ra_loader.c libsvn_ra_serf/options.c

[rh...@apache.org]

Author: rhuijben
Date: Wed Jan 28 20:34:26 2015
New Revision: 1655458

URL: http://svn.apache.org/r1655458
Log:
In the implementation of svn_ra_open4(), properly pass corrected_url as NULL
to the ra implementations, to allow them to error out early when users don't
want to support redirects.

Before this patch we sometimes returned an RA session pointing to a location
that would only respond with redirect errors on individual ra operations.
(We would also assume that the repository was using our http 1.0 protocol)

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_open4): Directly pass corrected_url_p to the implementations and
    move the code to fixup bad urls into serf. (Which should follow the
    Subversion API rules of always passing good urls anyway)

* subversion/libsvn_ra_serf/options.c
  (includes): Add svn_path.h.
  (svn_ra_serf__exchange_capabilities): Guarantee a proper redirect url.

Modified:
    subversion/trunk/subversion/libsvn_ra/ra_loader.c
    subversion/trunk/subversion/libsvn_ra_serf/options.c

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=1655458&r1=1655457&r2=1655458&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Wed Jan 28 20:34:26 2015
@@ -279,7 +279,6 @@ svn_error_t *svn_ra_open4(svn_ra_session
   svn_boolean_t store_pp = SVN_CONFIG_DEFAULT_OPTION_STORE_SSL_CLIENT_CERT_PP;
   const char *store_pp_plaintext
     = SVN_CONFIG_DEFAULT_OPTION_STORE_SSL_CLIENT_CERT_PP_PLAINTEXT;
-  const char *corrected_url;
 
   /* Initialize the return variable. */
   *session_p = NULL;
@@ -482,7 +481,8 @@ svn_error_t *svn_ra_open4(svn_ra_session
   session->pool = sesspool;
 
   /* Ask the library to open the session. */
-  err = vtable->open_session(session, &corrected_url, repos_URL,
+  err = vtable->open_session(session, corrected_url_p,
+                             repos_URL,
                              callbacks, callback_baton, config, sesspool);
 
   if (err)
@@ -495,21 +495,9 @@ svn_error_t *svn_ra_open4(svn_ra_session
      correction (a 301 or 302 redirect response during the initial
      OPTIONS request), then kill the session so the caller can decide
      what to do. */
-  if (corrected_url_p && corrected_url)
+  if (corrected_url_p && *corrected_url_p)
     {
-      if (! svn_path_is_url(corrected_url))
-        {
-          /* RFC1945 and RFC2616 state that the Location header's
-             value (from whence this CORRECTED_URL ultimately comes),
-             if present, must be an absolute URI.  But some Apache
-             versions (those older than 2.2.11, it seems) transmit
-             only the path portion of the URI.  See issue #3775 for
-             details. */
-          apr_uri_t corrected_URI = repos_URI;
-          corrected_URI.path = (char *)corrected_url;
-          corrected_url = apr_uri_unparse(pool, &corrected_URI, 0);
-        }
-      *corrected_url_p = svn_uri_canonicalize(corrected_url, pool);
+      /* *session_p = NULL; */
       svn_pool_destroy(sesspool);
       return SVN_NO_ERROR;
     }

Modified: subversion/trunk/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/options.c?rev=1655458&r1=1655457&r2=1655458&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/options.c Wed Jan 28 20:34:26 2015
@@ -30,6 +30,7 @@
 #include "svn_dirent_uri.h"
 #include "svn_hash.h"
 #include "svn_pools.h"
+#include "svn_path.h"
 #include "svn_ra.h"
 #include "svn_dav.h"
 #include "svn_xml.h"
@@ -522,7 +523,33 @@ svn_ra_serf__exchange_capabilities(svn_r
      successfully parsing as XML or somesuch. */
   if (corrected_url && (opt_ctx->handler->sline.code == 301))
     {
-      *corrected_url = apr_pstrdup(result_pool, opt_ctx->handler->location);
+      if (!opt_ctx->handler->location || !*opt_ctx->handler->location)
+        {
+          return svn_error_create(
+                    SVN_ERR_RA_DAV_RESPONSE_HEADER_BADNESS, NULL,
+                    _("Location header not set on redirect response"));
+        }
+      else if (svn_path_is_url(opt_ctx->handler->location))
+        {
+          *corrected_url = svn_uri_canonicalize(opt_ctx->handler->location,
+                                                result_pool);
+        }
+      else
+        {
+          /* RFC1945 and RFC2616 state that the Location header's value
+             (from whence this CORRECTED_URL comes), if present, must be an
+             absolute URI.  But some Apache versions (those older than 2.2.11,
+             it seems) transmit only the path portion of the URI.
+             See issue #3775 for details. */
+
+          apr_uri_t corrected_URI = serf_sess->session_url;
+
+          corrected_URI.path = (char *)corrected_url;
+          *corrected_url = svn_uri_canonicalize(
+                              apr_uri_unparse(scratch_pool, &corrected_URI, 0),
+                              result_pool);
+        }
+
       return SVN_NO_ERROR;
     }