You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2020/02/01 17:58:47 UTC

svn commit: r1873487 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_ra/ libsvn_ra_local/ libsvn_ra_serf/ libsvn_ra_svn/

Author: stsp
Date: Sat Feb  1 17:58:47 2020
New Revision: 1873487

URL: http://svn.apache.org/viewvc?rev=1873487&view=rev
Log:
Make it possible for RA-layer consumers to detect redirect loops reliably.

Make svn_ra_open() return both canonicalized and non-canonicalized versions
of redirect URLs. The latter can be used to detect redirect loops, while
the former can be used for any other purposes as usual.

See r1866899 and r1873375 for reasons why this is necessary.

* subversion/include/svn_ra.h
  (svn_ra_open4): Deprecate. 
  (svn_ra_open5): Declare.

* subversion/libsvn_client/ra.c
  (svn_client__open_ra_session_internal): Detect redirect loops based on the
   non-canonicalized representation of the URL used by the RA implementation.
   The canonicalized version is insufficient for loop detection.
  
* subversion/libsvn_ra/deprecated.c
  (svn_ra_open4): Implement as a wrapper around svn_ra_open5().

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_open5): Implement.
  (svn_ra_create_callbacks): Update for new output parameter 'redirect_url_p'.


* subversion/libsvn_ra/ra_loader.h
  (open_session): Needs a new output parameter.

* subversion/libsvn_ra/wrapper_template.h
  (compat_open): Track addition of new output parameter.

* subversion/libsvn_ra_local/ra_plugin.c
  (svn_ra_local__open): Add new output parameter.

* subversion/libsvn_ra_serf/options.c
  (svn_ra_serf__exchange_capabilities): Return the URL received in a HTTP
   redirect as-is in new 'redirect_url' output parameter.
  (svn_ra_serf__has_capability): Track addition of output parameter.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__exchange_capabilities): Declare new output parameter.

* subversion/libsvn_ra_serf/serf.c
  (svn_ra_serf__open): Support new 'redirect_url' output parameter.

* subversion/libsvn_ra_svn/client.c
  (ra_svn_open): Support new 'redirect_url' output parameter.

Modified:
    subversion/trunk/subversion/include/svn_ra.h
    subversion/trunk/subversion/libsvn_client/ra.c
    subversion/trunk/subversion/libsvn_ra/deprecated.c
    subversion/trunk/subversion/libsvn_ra/ra_loader.c
    subversion/trunk/subversion/libsvn_ra/ra_loader.h
    subversion/trunk/subversion/libsvn_ra/wrapper_template.h
    subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
    subversion/trunk/subversion/libsvn_ra_serf/options.c
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/serf.c
    subversion/trunk/subversion/libsvn_ra_svn/client.c

Modified: subversion/trunk/subversion/include/svn_ra.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_ra.h?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_ra.h (original)
+++ subversion/trunk/subversion/include/svn_ra.h Sat Feb  1 17:58:47 2020
@@ -65,7 +65,7 @@ svn_ra_version(void);
  * @a close_baton as appropriate.
  *
  * @a path is relative to the "root" of the session, defined by the
- * @a repos_URL passed to svn_ra_open4() vtable call.
+ * @a repos_URL passed to svn_ra_open5() vtable call.
  *
  * @a name is the name of the property to fetch. If the property is present,
  * then it is returned in @a value. Otherwise, @a *value is set to @c NULL.
@@ -369,7 +369,7 @@ typedef struct svn_ra_reporter3_t
    * implementor should assume the directory has no entries or props.
    *
    * This will *override* any previous set_path() calls made on parent
-   * paths.  @a path is relative to the URL specified in svn_ra_open4().
+   * paths.  @a path is relative to the URL specified in svn_ra_open5().
    *
    * If @a lock_token is non-NULL, it is the lock token for @a path in the WC.
    *
@@ -520,7 +520,7 @@ typedef struct svn_ra_reporter_t
 /** A collection of callbacks implemented by libsvn_client which allows
  * an RA layer to "pull" information from the client application, or
  * possibly store information.  libsvn_client passes this vtable to
- * svn_ra_open4().
+ * svn_ra_open5().
  *
  * Each routine takes a @a callback_baton originally provided with the
  * vtable.
@@ -710,6 +710,14 @@ typedef struct svn_ra_session_t svn_ra_s
  * within the new repository root URL that @a repos_URL pointed to within
  * the old repository root URL.
  *
+ * If @a redirect_url is not NULL and a @corrected_url is returned, then
+ * @a redirect_url contains a non-canonicalized version of @a corrected_url,
+ * as communicated in the network protocol used by the RA provider.
+ * THe @a redirect_url should be used for to detect redirection loops.
+ * Canonicalization may change the protocol-level URL in a way that
+ * makes detection of redirect loops impossible in some cases since URLs which
+ * are different at the protocol layer could map to the same canonicalized URL.
+ *
  * Return @c SVN_ERR_RA_UUID_MISMATCH if @a uuid is non-NULL and not equal
  * to the UUID of the repository at @c repos_URL.
  *
@@ -728,7 +736,24 @@ typedef struct svn_ra_session_t svn_ra_s
  *
  * @see svn_client_open_ra_session().
  *
+ * @since New in 1.14.
+ */
+svn_error_t *
+svn_ra_open5(svn_ra_session_t **session_p,
+             const char **corrected_url,
+             const char **redirect_url,
+             const char *repos_URL,
+             const char *uuid,
+             const svn_ra_callbacks2_t *callbacks,
+             void *callback_baton,
+             apr_hash_t *config,
+             apr_pool_t *pool);
+
+/** Similar to svn_ra_open5(), but with @a redirect_url always passed
+ * as @c NULL.
+ *
  * @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.13 API.
  */
 svn_error_t *
 svn_ra_open4(svn_ra_session_t **session_p,

Modified: subversion/trunk/subversion/libsvn_client/ra.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/ra.c (original)
+++ subversion/trunk/subversion/libsvn_client/ra.c Sat Feb  1 17:58:47 2020
@@ -402,8 +402,7 @@ svn_client__open_ra_session_internal(svn
         }
     }
 
-  /* If the caller allows for auto-following redirections, and the
-     RA->open() call above reveals a CORRECTED_URL, try the new URL.
+  /* If the caller allows for auto-following redirections, try the new URL.
      We'll do this in a loop up to some maximum number follow-and-retry
      attempts.  */
   if (corrected_url)
@@ -414,12 +413,14 @@ svn_client__open_ra_session_internal(svn
       *corrected_url = NULL;
       while (attempts_left--)
         {
-          const char *corrected = NULL;
+          const char *corrected = NULL; /* canonicalized version */
+          const char *redirect_url = NULL; /* non-canonicalized version */
 
           /* Try to open the RA session.  If this is our last attempt,
              don't accept corrected URLs from the RA provider. */
-          SVN_ERR(svn_ra_open4(ra_session,
+          SVN_ERR(svn_ra_open5(ra_session,
                                attempts_left == 0 ? NULL : &corrected,
+                               attempts_left == 0 ? NULL : &redirect_url,
                                base_url, uuid, cbtable, cb, ctx->config,
                                result_pool));
 
@@ -441,13 +442,22 @@ svn_client__open_ra_session_internal(svn
           *corrected_url = corrected;
 
           /* Make sure we've not attempted this URL before. */
-          if (svn_hash_gets(attempted, corrected))
+          if (svn_hash_gets(attempted, redirect_url))
             return svn_error_createf(SVN_ERR_CLIENT_CYCLE_DETECTED, NULL,
                                      _("Redirect cycle detected for URL '%s'"),
-                                     corrected);
+                                     redirect_url);
+
+          /*
+           * Remember this redirect URL so we don't wind up in a loop.
+           *
+           * Store the non-canonicalized version of the URL. The canonicalized
+           * version is insufficient for loop detection because we might not get
+           * an exact match against URLs used by the RA protocol-layer (the URL
+           * used by the protocol may contain trailing slashes, for example,
+           * which are stripped during canonicalization).
+           */
+          svn_hash_sets(attempted, redirect_url, (void *)1);
 
-          /* Remember this CORRECTED_URL so we don't wind up in a loop. */
-          svn_hash_sets(attempted, corrected, (void *)1);
           base_url = corrected;
         }
     }

Modified: subversion/trunk/subversion/libsvn_ra/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/deprecated.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_ra/deprecated.c Sat Feb  1 17:58:47 2020
@@ -151,6 +151,19 @@ static svn_ra_reporter2_t reporter_3in2_
   abort_report
 };
 
+svn_error_t *svn_ra_open4(svn_ra_session_t **session_p,
+                          const char **corrected_url_p,
+                          const char *repos_URL,
+                          const char *uuid,
+                          const svn_ra_callbacks2_t *callbacks,
+                          void *callback_baton,
+                          apr_hash_t *config,
+                          apr_pool_t *pool)
+{
+  return svn_ra_open5(session_p, corrected_url_p, NULL, repos_URL, uuid,
+                      callbacks, callback_baton, config, pool);
+}
+
 svn_error_t *svn_ra_open3(svn_ra_session_t **session_p,
                           const char *repos_URL,
                           const char *uuid,

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Sat Feb  1 17:58:47 2020
@@ -256,8 +256,9 @@ svn_ra_create_callbacks(svn_ra_callbacks
   return SVN_NO_ERROR;
 }
 
-svn_error_t *svn_ra_open4(svn_ra_session_t **session_p,
+svn_error_t *svn_ra_open5(svn_ra_session_t **session_p,
                           const char **corrected_url_p,
+                          const char **redirect_url_p,
                           const char *repos_URL,
                           const char *uuid,
                           const svn_ra_callbacks2_t *callbacks,
@@ -381,7 +382,7 @@ 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_p,
+  err = vtable->open_session(session, corrected_url_p, redirect_url_p,
                              repos_URL,
                              callbacks, callback_baton, auth_baton,
                              config, sesspool, scratch_pool);
@@ -406,6 +407,8 @@ svn_error_t *svn_ra_open4(svn_ra_session
     {
       /* *session_p = NULL; */
       *corrected_url_p = apr_pstrdup(pool, *corrected_url_p);
+      if (redirect_url_p && *redirect_url_p)
+        *redirect_url_p = apr_pstrdup(pool, *redirect_url_p);
       svn_pool_destroy(sesspool); /* Includes scratch_pool */
       return SVN_NO_ERROR;
     }

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.h?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.h (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.h Sat Feb  1 17:58:47 2020
@@ -64,11 +64,12 @@ typedef struct svn_ra__vtable_t {
 
   /* Implementations of the public API functions. */
 
-  /* See svn_ra_open4(). */
+  /* See svn_ra_open5(). */
   /* All fields in SESSION, except priv, have been initialized by the
      time this is called.  SESSION->priv may be set by this function. */
   svn_error_t *(*open_session)(svn_ra_session_t *session,
                                const char **corrected_url,
+                               const char **redirect_url,
                                const char *session_URL,
                                const svn_ra_callbacks2_t *callbacks,
                                void *callback_baton,

Modified: subversion/trunk/subversion/libsvn_ra/wrapper_template.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/wrapper_template.h?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/wrapper_template.h (original)
+++ subversion/trunk/subversion/libsvn_ra/wrapper_template.h Sat Feb  1 17:58:47 2020
@@ -90,7 +90,7 @@ static svn_error_t *compat_open(void **s
   callbacks2->progress_func = NULL;
   callbacks2->progress_baton = NULL;
 
-  SVN_ERR(VTBL.open_session(sess, &session_url, repos_URL,
+  SVN_ERR(VTBL.open_session(sess, &session_url, NULL, repos_URL,
                             callbacks2, callback_baton,
                             callbacks ? callbacks->auth_baton : NULL,
                             config, sesspool, sesspool));

Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
+++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Sat Feb  1 17:58:47 2020
@@ -554,6 +554,7 @@ ignore_warnings(void *baton,
 static svn_error_t *
 svn_ra_local__open(svn_ra_session_t *session,
                    const char **corrected_url,
+                   const char **redirect_url,
                    const char *repos_URL,
                    const svn_ra_callbacks2_t *callbacks,
                    void *callback_baton,
@@ -576,6 +577,8 @@ svn_ra_local__open(svn_ra_session_t *ses
   /* We don't support redirections in ra-local. */
   if (corrected_url)
     *corrected_url = NULL;
+  if (redirect_url)
+    *redirect_url = NULL;
 
   /* Allocate and stash the session_sess args we have already. */
   sess = apr_pcalloc(pool, sizeof(*sess));

Modified: subversion/trunk/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/options.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/options.c Sat Feb  1 17:58:47 2020
@@ -546,6 +546,7 @@ svn_ra_serf__v1_get_activity_collection(
 svn_error_t *
 svn_ra_serf__exchange_capabilities(svn_ra_serf__session_t *serf_sess,
                                    const char **corrected_url,
+                                   const char **redirect_url,
                                    apr_pool_t *result_pool,
                                    apr_pool_t *scratch_pool)
 {
@@ -553,6 +554,8 @@ svn_ra_serf__exchange_capabilities(svn_r
 
   if (corrected_url)
     *corrected_url = NULL;
+  if (redirect_url)
+    *redirect_url = NULL;
 
   /* This routine automatically fills in serf_sess->capabilities */
   SVN_ERR(create_options_req(&opt_ctx, serf_sess, scratch_pool));
@@ -577,6 +580,9 @@ svn_ra_serf__exchange_capabilities(svn_r
         {
           SVN_ERR(svn_uri_canonicalize_safe(corrected_url, NULL,
               opt_ctx->handler->location, result_pool, scratch_pool));
+          if (redirect_url)
+            *redirect_url = apr_pstrdup(result_pool,
+                                        opt_ctx->handler->location);
         }
       else
         {
@@ -593,6 +599,8 @@ svn_ra_serf__exchange_capabilities(svn_r
           absolute_uri = apr_uri_unparse(scratch_pool, &corrected_URI, 0);
           SVN_ERR(svn_uri_canonicalize_safe(corrected_url, NULL,
               absolute_uri, result_pool, scratch_pool));
+          if (redirect_url)
+            *redirect_url = apr_pstrdup(result_pool, absolute_uri);
         }
 
       return SVN_NO_ERROR;
@@ -700,7 +708,8 @@ svn_ra_serf__has_capability(svn_ra_sessi
 
   /* If any capability is unknown, they're all unknown, so ask. */
   if (cap_result == NULL)
-    SVN_ERR(svn_ra_serf__exchange_capabilities(serf_sess, NULL, pool, pool));
+    SVN_ERR(svn_ra_serf__exchange_capabilities(serf_sess, NULL, NULL,
+                                               pool, pool));
 
   /* Try again, now that we've fetched the capabilities. */
   cap_result = svn_hash_gets(serf_sess->capabilities, capability);

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Sat Feb  1 17:58:47 2020
@@ -1472,6 +1472,7 @@ svn_ra_serf__get_mergeinfo(svn_ra_sessio
 svn_error_t *
 svn_ra_serf__exchange_capabilities(svn_ra_serf__session_t *serf_sess,
                                    const char **corrected_url,
+                                   const char **redirect_url,
                                    apr_pool_t *result_pool,
                                    apr_pool_t *scratch_pool);
 

Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Sat Feb  1 17:58:47 2020
@@ -476,6 +476,7 @@ get_user_agent_string(apr_pool_t *pool)
 static svn_error_t *
 svn_ra_serf__open(svn_ra_session_t *session,
                   const char **corrected_url,
+                  const char **redirect_url,
                   const char *session_URL,
                   const svn_ra_callbacks2_t *callbacks,
                   void *callback_baton,
@@ -492,6 +493,8 @@ svn_ra_serf__open(svn_ra_session_t *sess
 
   if (corrected_url)
     *corrected_url = NULL;
+  if (redirect_url)
+    *redirect_url = NULL;
 
   serf_sess = apr_pcalloc(result_pool, sizeof(*serf_sess));
   serf_sess->pool = result_pool;
@@ -599,6 +602,7 @@ svn_ra_serf__open(svn_ra_session_t *sess
   serf_sess->conn_latency = -1;
 
   err = svn_ra_serf__exchange_capabilities(serf_sess, corrected_url,
+                                           redirect_url,
                                            result_pool, scratch_pool);
 
   /* serf should produce a usable error code instead of APR_EGENERAL */

Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=1873487&r1=1873486&r2=1873487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/client.c Sat Feb  1 17:58:47 2020
@@ -841,6 +841,7 @@ is_valid_hostinfo(const char *hostinfo)
 
 static svn_error_t *ra_svn_open(svn_ra_session_t *session,
                                 const char **corrected_url,
+                                const char **redirect_url,
                                 const char *url,
                                 const svn_ra_callbacks2_t *callbacks,
                                 void *callback_baton,
@@ -858,6 +859,8 @@ static svn_error_t *ra_svn_open(svn_ra_s
   /* We don't support server-prescribed redirections in ra-svn. */
   if (corrected_url)
     *corrected_url = NULL;
+  if (redirect_url)
+    *redirect_url = NULL;
 
   SVN_ERR(parse_url(url, &uri, sess_pool));
 
@@ -913,7 +916,7 @@ static svn_error_t *ra_svn_dup_session(s
 {
   svn_ra_svn__session_baton_t *old_sess = old_session->priv;
 
-  SVN_ERR(ra_svn_open(new_session, NULL, new_session_url,
+  SVN_ERR(ra_svn_open(new_session, NULL, NULL, new_session_url,
                       old_sess->callbacks, old_sess->callbacks_baton,
                       old_sess->auth_baton, old_sess->config,
                       result_pool, scratch_pool));