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

Re: [PATCH] [Issue 1942] svn+ssh should use the port value

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2006-02-08 12:17:18 CET

Hi Eric,
Thanks for your comments.
Have corrected the patch to take care of the case projected by you.

This -p solution is only for 'ssh (openssh)' client which we assume by
default in absence of ssh tunnel definition.
This fix is not available if the user has custom 'ssh' tunnel defintions.

With regards
Kamesh Jayachandran

[[[
Fix issue #1942, svn+ssh should use the port value.

Patch by: Kamesh Jayachandran <kamesh@collab.net>

* subversion/libsvn_ra_svn/client.c
  (find_tunnel_agent): Modified to accept the apr_uri_t*
  rather than hostinfo so that we can make use of the
  port information available in apr_uri_t.
  This port is given as -p to ssh binary in case available.
  This feature is not effective if the user has some custom
  'ssh' tunnel defined already, so it won't override the user
   custom settings.
  (ra_svn_open): calls this new version of find_tunnel_agent.
* subversion/libsvn_subr/config_file.c
  (svn_config_ensure): Updated comments in ~/.subversion/config
  template to mention about the viablity of hostname:port url style.
]]]

Eric Gillespie wrote:
> Kamesh Jayachandran <kamesh@collab.net> writes:
>
>
>> memcpy(*argv, cmd_argv, n * sizeof(char *));
>> - (*argv)[n++] = svn_path_uri_decode (hostinfo, pool);
>> + if (uri->port_str != NULL)
>> + {
>> + (*argv)[n++] = "-p";
>> + (*argv)[n++] = uri->port_str;
>> + }
>> + (*argv)[n++] = svn_path_uri_decode (uri->hostname, pool);
>>
>
> Unless i misread this, your patch breaks every agent
> configuration except certain ssh or plink ones, or any other
> agent definition that would happen to work with a '-p' appended
> to it.
>
> -1
>
>

Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 18390)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -1131,6 +1131,30 @@
         APR_EOL_STR
         "# ssh = $SVN_SSH ssh"
         APR_EOL_STR
+ "### The built-in ssh scheme assumes the ssh tunnel binary to use for"
+ APR_EOL_STR
+ "### tunnelling as 'ssh'. In case you want to access the repository"
+ APR_EOL_STR
+ "### with some other ssh tunnel agent say 'plink' define tunnel "
+ APR_EOL_STR
+ "### as follows,"
+ APR_EOL_STR
+ "# ssh = $SVN_SSH plink"
+ APR_EOL_STR
+ "### If the URL given is of form hostname:port, built-in ssh scheme"
+ APR_EOL_STR
+ "### invokes the default ssh tunnel agent 'ssh' with -p port as 'ssh'"
+ APR_EOL_STR
+ "### understands it. If your ssh tunnel agent does not accept '-p' "
+ APR_EOL_STR
+ "### switch for port or it does not understand URL of the form"
+ APR_EOL_STR
+ "### hostname:port, you should use custom tunnel definition as"
+ APR_EOL_STR
+ "### demonstrated for 'plink' below."
+ APR_EOL_STR
+ "# ssh = $SVN_SSH plink -P port_ssh_server_listens"
+ APR_EOL_STR
         "### If you wanted to define a new 'rsh' scheme, to be used with"
         APR_EOL_STR
         "### 'svn+rsh:' URLs, you could do so as follows:"
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 18390)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -439,7 +439,7 @@
 /* --- RA LAYER IMPLEMENTATION --- */
 
 static svn_error_t *find_tunnel_agent(const char *tunnel,
- const char *hostinfo,
+ const apr_uri_t *uri,
                                       const char ***argv,
                                       apr_hash_t *config, apr_pool_t *pool)
 {
@@ -457,7 +457,22 @@
 
   /* We have one predefined tunnel scheme, if it isn't overridden by config. */
   if (!val && strcmp(tunnel, "ssh") == 0)
- val = "$SVN_SSH ssh";
+ {
+ char *default_ssh_tunnel = "$SVN_SSH ssh";
+ if (uri->port_str == NULL)
+ {
+ val = default_ssh_tunnel;
+ }
+ else
+ {
+ //5 is for ' -p ' + null ending
+ val = apr_palloc(pool, strlen(default_ssh_tunnel) + 5 +
+ strlen(uri->port_str));
+ strcpy(val, default_ssh_tunnel);
+ strcat(val, " -p ");
+ strcat(val, uri->port_str);
+ }
+ }
 
   if (!val || !*val)
     return svn_error_createf(SVN_ERR_BAD_URL, NULL,
@@ -496,7 +511,7 @@
     ;
   *argv = apr_palloc(pool, (n + 4) * sizeof(char *));
   memcpy(*argv, cmd_argv, n * sizeof(char *));
- (*argv)[n++] = svn_path_uri_decode (hostinfo, pool);
+ (*argv)[n++] = svn_path_uri_decode (uri->hostname, pool);
   (*argv)[n++] = "svnserve";
   (*argv)[n++] = "-t";
   (*argv)[n] = NULL;
@@ -719,8 +734,7 @@
   parse_tunnel (url, &tunnel, pool);
 
   if (tunnel)
- SVN_ERR(find_tunnel_agent(tunnel, uri.hostinfo, &tunnel_argv, config,
- pool));
+ SVN_ERR(find_tunnel_agent(tunnel, &uri, &tunnel_argv, config, pool));
   else
     tunnel_argv = NULL;
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 8 12:17:57 2006

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.