flac icon indicating copy to clipboard operation
flac copied to clipboard

Flac clobbers filenames that contain a backslash.

Open orbea opened this issue 7 years ago • 14 comments

When filenames contain a backslash (\) flac will clobber the output filename.

Example:

  1. cp test.flac te\\st.flac
  2. flac --decode te\\st.flac
  3. Creates te\st.flac and prints...
flac 1.3.2
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

st.flac: done

Or.

  1. flac te\\st.wav
  2. Creates st.flac and prints...
flac 1.3.2
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

st.wav: wrote 29732477 bytes, ratio=0.686

Interestingly with --decode the created file has the correct output filename, but when encoding a wav file the created file is wrong in addition to the incorrect verbose output.

OS: Slackware64-current Version: flac 1.3.2

orbea avatar May 30 '18 03:05 orbea

@erikd Any comments on this issue? I realizes its just cosmetic and niche, but would be nice to have it fixed.

orbea avatar Jun 24 '20 12:06 orbea

This is not actually FLAC doing this. This is the default behavior of your shell.

erikd avatar Jun 24 '20 20:06 erikd

Yeah. Try first "cp test.flac 'te\st.flac'" and then "flac --decode 'te\st.flac'". Use single quotes, not double quotes.

Here is an example to give you an idea what is going on:

% echo 'te\\st.flac'
te\\st.flac
% echo te\\st.flac
te\st.flac
% echo "te\\st.flac"
te\st.flac
%

petterreinholdtsen avatar Jun 24 '20 21:06 petterreinholdtsen

@erikd I don't think that makes sense.

  • It wouldn't explain why when decoding files it only affects the stdout and not the actual output file.
  • It wouldn't explain the difference in behavior when encoding files from decoding files.
  • It wouldn't explain why its the same with multiple different shells.

@petterreinholdtsen The example only has one /, not two. The first one escapes the second one making it literal. The same thing happens when using two forward slashes.

$ cp test.flac 'te\st.flac'
$ flac --decode 'te\\st.flac'

flac 1.3.3
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

st.flac: done

$ stat 'te\\st.wav'
  File: te\\st.wav
  Size: 43311148  	Blocks: 84600      IO Block: 4096   regular file
Device: 21h/33d	Inode: 38670735    Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/   username)   Gid: (  100/   users)
Access: 2020-06-24 20:18:51.000000000 -0700
Modify: 2020-06-24 20:17:48.000000000 -0700
Change: 2020-06-24 20:18:51.871943268 -0700
 Birth: -

orbea avatar Jun 25 '20 03:06 orbea

@erikd Do you have a problem with my analysis?

orbea avatar Jul 19 '20 13:07 orbea

The problem is that FLAC tries to strip the filename of the directory, and when you encode te\st.wav, that looks like file st.wav in directory te on Windows.

When you run command flac te\\st.flac the progress indicator shows st.flac When you run command flac ./te\\st.flac the progress indicator shows te\st.flac correctly

FLAC searches the filename it gets for the last slash it can find. If it doesn´t find a slash, it will look for the last backslash it can find. See the code here:

https://github.com/xiph/flac/blob/37d1a620eb2c2ffe52446df5832608196d57f562/src/share/grabbag/file.c#L81-L92

Perhaps it can be solved by putting line 87 in #if defined _WIN32 && !defined __CYGWIN__1 or something similar? To me it seems a minor issue though.

ktmf01 avatar Jul 08 '21 18:07 ktmf01

Thank you for looking into this, I tried adding #if defined _WIN32 && !defined __CYGWIN__1 guarding lines 87-88, but it did not make a difference.

Additionally at some point presumably between my last comment and now the difference between decoding and encoding files vanished where both produce the correct filename now. This issue is currently entirely cosmetic and I agree that it is minor, although it would be nice to see it fixed.

orbea avatar Jul 08 '21 19:07 orbea

@orbea I've tried it as well, and it seems to work fine here on a non-Windows machine. (See #390) With that, I get

$ src/flac/flac -t te\\st.flac

flac git-7e0a0e57 20220701
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

te\st.flac: ok

ktmf01 avatar Jul 12 '22 19:07 ktmf01

@ktmf01 Strange, I wonder what the difference is? This is on Gentoo, I tried in bash, dash, mksh, yash and zsh with no differences in results.

$ flac/src/flac/flac -t te\\st.flac 

flac git-772efde6 20220707
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

st.flac: ok

orbea avatar Jul 12 '22 19:07 orbea

That is strange. I tested on debian with bash. Assuming your file is called 'te\st.flac', shouldn't the command line be $ flac/src/flac/flac -t te\\st.flac?

At my end, te\st.flac is decoded simply to test.flac, which works if I have a file with that name

ktmf01 avatar Jul 12 '22 20:07 ktmf01

Yes, it should be te\\st.flac, but I lost one of the / when I pasted it on github and failed to notice. I edited my earlier comment

orbea avatar Jul 12 '22 20:07 orbea

Just a shot in the dark here, maybe libtool for some reasons doesn't use the freshly compiled lib but an installed lib instead?

Could you try compiling with

git clean -ffxd
./autogen.sh
./configure --enable-static --disable-shared
make LDFLAGS='-all-static'

And then try again just to be sure?

ktmf01 avatar Jul 12 '22 20:07 ktmf01

No difference unfortunately.

As a side note make LDFLAGS='-all-static' fails to build with cannot find -logg, but I was able to work around that for the test by using --disable-ogg.

orbea avatar Jul 12 '22 23:07 orbea

I've merged my patch and don't know what to try next, so I'm marking this as 'unreproducible' and perhaps try again at some point.

ktmf01 avatar Jul 13 '22 07:07 ktmf01

Your shell is removing backslashes dude, not the flac program or library.

MarcusJohnson91 avatar Mar 29 '23 14:03 MarcusJohnson91

Your shell is removing backslashes dude, not the flac program or library.

I'm sincerely doubtful its the shell, but it also doesn't appear to have been a problem in flac. I tested this with the current git head (9ee21a0e68de3fd1a59cf5f420220155245576ca), a previously reproducible commit (772efde6) and my currently installed version (1.4.2) on both musl and glibc gentoo systems without being able to reproduce this anymore. I next tried it on an older Slackware 14.2 server with the same results.

Not sure, but I guess I will close it if no one can reproduce this anymore...

orbea avatar Mar 29 '23 16:03 orbea