rsync - Jan 2003 - unsafe_symlink change (Re: CVS update: rsync)

The patch from 2-1/2 years ago for changing copy-unsafe-links to follow
unsafe links on the destination side also included essentially this patch.
When I looked at it, however, I asked why in the world is unsafe_symlink()
doing strdup() in the first place.  I think you could get rid of the
calls to strdup() and the new local variables and possibly do a couple
casts inside the function instead.

- Dave


On Wed, Jan 15, 2003 at 12:49:44PM -0500, wayned@samba.org wrote to
rsync-cvs:
> Re: CVS update: rsync
> 
> Date:	Wed Jan 15 17:49:44 2003
> Author:	wayned
> 
> Update of /data/cvs/rsync
> In directory dp.samba.org:/tmp/cvs-serv3194
> 
> Modified Files:
> 	proto.h util.c 
> Log Message:
> Make unsafe_symlink() take const args so that we don't get any
> compiler warnings when calling it with a const char *.
> 
> 
> Revisions:
> proto.h		1.150 => 1.151
> 	http://www.samba.org/cgi-bin/cvsweb/rsync/proto.h?r1=1.150&r2=1.151
> util.c		1.117 => 1.118
> 	http://www.samba.org/cgi-bin/cvsweb/rsync/util.c?r1=1.117&r2=1.118
> _______________________________________________
> rsync-cvs mailing list
> rsync-cvs@lists.samba.org
> http://lists.samba.org/mailman/listinfo/rsync-cvs

Dave Dykstra on Thu 16/01 14:33 -0600:
> The patch from 2-1/2 years ago for changing copy-unsafe-links to follow
> unsafe links on the destination side also included essentially this patch.
just to be clear, without using copy-unsafe-links, rsync still copies
absolute symlinks which point out of the source tree, verbatim to the
destination (even if it ends up dangling on the destination), right?

On Thu, Jan 16, 2003 at 03:44:44PM -0500, Scott Mcdermott
wrote:
> Dave Dykstra on Thu 16/01 14:33 -0600:
> > The patch from 2-1/2 years ago for changing copy-unsafe-links to
follow
> > unsafe links on the destination side also included essentially this
patch.
> 
> just to be clear, without using copy-unsafe-links, rsync still copies
> absolute symlinks which point out of the source tree, verbatim to the
> destination (even if it ends up dangling on the destination), right?
Right, with the -l or --links option (implied by -a).

- Dave

On Thu, Jan 16, 2003 at 02:33:26PM -0600, Dave Dykstra
wrote:
> I think you could get rid of the calls to strdup() and the new local
> variables and possibly do a couple casts inside the function instead.
You wouldn't want to carve up the const strings with strtok() (which
adds nulls to the strings).  The function could be rewritten to scan the
string for '/'s without using strtok(), which would be a more efficient
way to go (eliminating the need for the strdups).

Here's a quick rewrite that I minimally tested.  See if you like it.

..wayne..

---8<------8<------8<------8<---cut
here--->8------>8------>8------>8---
Index: util.c
--- util.c	15 Jan 2003 17:49:44 -0000	1.118
+++ util.c	17 Jan 2003 01:26:36 -0000
@@ -793,49 +793,38 @@
  *
  * @sa t_unsafe.c
  **/
-int unsafe_symlink(const char *dest_path, const char *src_path)
+int unsafe_symlink(const char *dest, const char *src)
 {
-	char *tok, *src, *dest;
+	const char *name, *slash;
 	int depth = 0;
 
 	/* all absolute and null symlinks are unsafe */
-	if (!dest_path || !*dest_path || *dest_path == '/') return 1;
-
-	src = strdup(src_path);
-	if (!src) out_of_memory("unsafe_symlink");
+	if (!dest || !*dest || *dest == '/') return 1;
 
 	/* find out what our safety margin is */
-	for (tok=strtok(src,"/"); tok; tok=strtok(NULL,"/")) {
-		if (strcmp(tok,"..") == 0) {
+	for (name = src; (slash = strchr(name, '/')); name = slash+1) {
+		if (strncmp(name, "../", 3) == 0) {
 			depth=0;
-		} else if (strcmp(tok,".") == 0) {
+		} else if (strncmp(name, "./", 2) == 0) {
 			/* nothing */
 		} else {
 			depth++;
 		}
 	}
-	free(src);
-
-	/* drop by one to account for the filename portion */
-	depth--;
-
-	dest = strdup(dest_path);
-	if (!dest) out_of_memory("unsafe_symlink");
 
-	for (tok=strtok(dest,"/"); tok; tok=strtok(NULL,"/")) {
-		if (strcmp(tok,"..") == 0) {
-			depth--;
-		} else if (strcmp(tok,".") == 0) {
+	for (name = dest; (slash = strchr(name, '/')); name = slash+1) {
+		if (strncmp(name, "../", 3) == 0) {
+			/* if at any point we go outside the current directory
+			   then stop - it is unsafe */
+			if (--depth < 0)
+				break;
+		} else if (strncmp(name, "./", 2) == 0) {
 			/* nothing */
 		} else {
 			depth++;
 		}
-		/* if at any point we go outside the current directory then
-		   stop - it is unsafe */
-		if (depth < 0) break;
 	}
 
-	free(dest);
 	return (depth < 0);
 }
 
---8<------8<------8<------8<---cut
here--->8------>8------>8------>8---