File name too long

[jw schultz]

On Mon, Mar 10, 2003 at 05:09:56PM +0100, Paul Slootman wrote:
> On Mon 10 Mar 2003, jw schultz wrote:
> > 
> > What you have done is hit a file created by something insane
> > where the filename is 252 characters long.  Adding the
> > prepended . and the appended .XXXXXX made it 260 characters
> > long and the filesystem doesn't support that long a name.
> 
> Correct analysis.
> 
> > I have a recollection of someone suggesting a patch to
> > so our use of mkstemp didn't increase the length of long
> > filenames but i don't recall anything coming of that.
> > 
> > The easiest solution would be for the file creator to stop
> > making such long file names.  If you cannot do that i have
> 
> That's just fixing the symptoms, not the cause of the problem...

I agree. But it may well be the easiest for that user.

> > It isn't really a debian problem although maybe one of their
> > developers would create a better fix than i just did.
> 
> By a coincidence I'm currently looking at the open bugs on the Debian
> rsync package. I'm a Debian developer, although not the one that is in
> charge of rsync. That developer is apparently short of time so he can't
> properly maintain rsync at the moment, so I'm seeing whether I can fix
> anything in the meantime.
> 
> Here's the patch I came up with for this. I did a bit too much I guess;
> the get_tmpname function had most code duplicated in the branches with and
> without tmpdir, and I hate that so I eliminated it...  I also added
> comments (horror!)
> 
> What's not fixed is if the pathname of the tmpfile in the tmpdir is too
> long (without the added gunk for the template). I haven't looked into
> the tmpdir thing at all yet. However, I expect that that shouldn't be
> too difficult to fix as well, if it's even necessary.
> 
> With this patch, I can successfully transfer a file that has a 255-char
> filename.
> 
> Only thing that may be a problem is the NAME_MAX thing, but that's a
> POSIX thing, so should be relatively safe (famous last words).

Overall it looks like it should be an improvement.  Getting
rid of all that code duplication is a real gain.

The one thing that bothers me, also present in the current
code is the bit of changing and then restoring fname.  That
complicates the code in ways that are prone to induce human
error (a problem with my hack).  It would be better to just
use a scratch char array.

> 
> 
> Paul Slootman
> 
> diff -ru orig/rsync-2.5.6/receiver.c rsync-2.5.6/receiver.c
> --- orig/rsync-2.5.6/receiver.c	2003-01-21 00:32:17.000000000 +0100
> +++ rsync-2.5.6/receiver.c	2003-03-10 17:03:47.000000000 +0100
> @@ -166,37 +166,49 @@
>  
>  static int get_tmpname(char *fnametmp, char *fname)
>  {
> -	char *f;
> -
> -	/* open tmp file */
> -	if (tmpdir) {
> -		f = strrchr(fname,'/');
> -		if (f == NULL) 
> -			f = fname;
> -		else 
> -			f++;
> -		if (strlen(tmpdir)+strlen(f)+10 > MAXPATHLEN) {
> -			rprintf(FERROR,"filename too long\n");
> -			return 0;
> -		}
> -		snprintf(fnametmp,MAXPATHLEN, "%s/.%s.XXXXXX",tmpdir,f);
> -		return 1;
> -	} 
> -
> -	f = strrchr(fname,'/');
> +	char	*f;
> +	int	slash = 0;	/* restore the '/' ? */
> +	char	x = 0;		/* restore the shortened name (with this?) */
> +	char	*dir = "";	/* what dir to put the temp file in */
> +
> +	if (tmpdir)
> +		dir = tmpdir;
> +	f = strrchr(fname,'/');	/* is there a directory to skip in fname? */
> +	if (f == NULL) {
> +		f = fname;		/* no */
> +	}
> +	else {
> +		slash++;		/* yes */
> +		*f = 0;
> +		f++;
> +		dir = fname;
> +	}
>  
> -	if (strlen(fname)+9 > MAXPATHLEN) {
> +	if (strlen(dir)+strlen(f)+1 > MAXPATHLEN) {
>  		rprintf(FERROR,"filename too long\n");
>  		return 0;
>  	}
>  
> -	if (f) {
> -		*f = 0;
> -		snprintf(fnametmp,MAXPATHLEN,"%s/.%s.XXXXXX",
> -			 fname,f+1);
> +	if (strlen(f)+8 > NAME_MAX || strlen(dir)+strlen(f)+9 > MAXPATHLEN) {
> +		/* temporarily shorten the name, it's just for a temp name anyway */
> +		x = f[NAME_MAX-10];
> +		f[NAME_MAX-10] = 0;
> +	}
> +
> +	/* construct temp name template */
> +	if (*dir) {
> +		snprintf(fnametmp, MAXPATHLEN, "%s/.%s.XXXXXX", dir, f);
> +	} 
> +	else {
> +		snprintf(fnametmp, MAXPATHLEN, ".%s.XXXXXX", f);
> +	}
> +
> +	if (x)
> +		f[NAME_MAX-10] = x; /* restore the name if necessary */
> +
> +	if (slash) {	/* restore slash if necessary */
> +		f--;
>  		*f = '/';
> -	} else {
> -		snprintf(fnametmp,MAXPATHLEN,".%s.XXXXXX",fname);
>  	}
>  
>  	return 1;
> -- 
> To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
> Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html

-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt