Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MacOS] Downloading a torrent with symbolic links in it and libtorrent do not create them #3636

Closed
AlexanderVitiuk opened this issue Feb 13, 2019 · 32 comments
Assignees
Milestone

Comments

@AlexanderVitiuk
Copy link

libtorrent version: 1.2
os: macosx 10.13.6

I create torrent-file of some macosx framework, which have symlinks to internal folders (SDL2 Framework for example).
To create torrent-file i use make_torrent tool using -l key. I checked it by dump_torrent tool, and can see valid symlinks inside.
But when i try download data using that .torrent-file by libtorrent, it creates zero-sized files instead of symlinks...
Cannot find any related code in libtorrent sources, which create symlinks when downloading data.
Also cannot find any info about it in documentation, so i guess it is not a normal behavior.

(zipped torrent-file attached).
tt2macosx.zip

@cathalgarvey
Copy link

At a guess (I don't know if this is the real reason).. symlinks can be a security hazard to users, and they can add very unexpected behaviour for users. So if it were easy to do this, I think people probably wouldn't do it, anyway.

The bigger problem is that symlinks are not actually "files" with data in them. Symlinks are a feature of the file system they're made on, and I don't think there's any specified way to represent them in torrent files. As a Mac user, when you make a symlink, it's not the same as when I, a Linux user, make a symlink. The results look similar, but the reality is that there is no file to include.

Now, if you meant "I'd like libtorrent to include a file, as a normal file, when I link it into a folder".. at least on Linux, you could probably do that with a Hard Symlink, and Mac might have an equivalent to that? The result would be that, to Libtorrent, it would be seeing the full file everywhere that a "link" appeared. So if there were 10 links to the same file, it would hash the file ten times. And users would download the file again, ten times. And, unless the files aligned on piece boundaries perfectly, the pieces would also be different for all 10 copies. :)

@AlexanderVitiuk
Copy link
Author

As i can see in libtorrent documentation, it supports storing symlinks in torrent-files (there are special flags and fields in torrent_info class). So if it allows storing symlinks, i guess it must restoring it too, in other case what is the reason of it?..

@AlexanderVitiuk AlexanderVitiuk changed the title Downloading a torrent with symbolic links in it and libtorrent do not create them [MacOS] Downloading a torrent with symbolic links in it and libtorrent do not create them Feb 14, 2019
@arvidn
Copy link
Owner

arvidn commented Feb 15, 2019

looking through the code, I can only find support for creating and loading/parsing torrents with symlinks, not to actually create the symlinks.

I think a good place to do so would be in default_storage::initialize().

@AlexanderVitiuk would you be up for writing a test demonstrating this failing? (and I can implement support for it). I think extending test/test_torrent.cpp would be the simplest probably.

@arvidn arvidn self-assigned this Feb 15, 2019
@AlexanderVitiuk
Copy link
Author

Ok i will try, thanks for answer)

@AlexanderVitiuk
Copy link
Author

Finally i got free time to do it.
test.ZIP

Attached test.zip contains 2 files:
symlink2.torrent <--- put it into folder: test\test_torrents
test_symlinks.cpp <--- copy and paste its content to the end of test\test_torrent.cpp

@arvidn arvidn added this to the 1.2.1 milestone Feb 25, 2019
@arvidn
Copy link
Owner

arvidn commented Feb 26, 2019

thanks! I'll make some tweaks to not actually download anything as part of the test.

btw, did libtorrent actually create that torrent? I would be a bit surprised, since the name of the torrent had quotes in it. Perhaps the windows shell doesn't strip quotes.

@AlexanderVitiuk
Copy link
Author

I used compiled example tool "make_torrent" to create it (on macosx). It called from python script:
...
from subprocess import call
...
command = [
'./make_torrent',
config.DST_PATH, '-l',
'-w', config.WEB_SEED,
'-c', '"' + config.COMMENT + '"',
'-o', config.TORRENT_FILE,
]
for tracker in config.TRACKERS:
command += ['-t', tracker]
call(command, stdout=sys.stdout, stderr=sys.stdout)
...

@arvidn
Copy link
Owner

arvidn commented Feb 26, 2019

I see. so you added the quotes yourself.

anyway, please give this patch a try! It's against RC_1_2. #3684

@arvidn
Copy link
Owner

arvidn commented Mar 4, 2019

@AlexanderVitiuk have you had a chance to try this out?

@AlexanderVitiuk
Copy link
Author

AlexanderVitiuk commented Mar 4, 2019

No, sorry, i hope i will try today or tomorrow.

@AlexanderVitiuk
Copy link
Author

Checked on test torrent with symlinks, seem it is still not working (still create zero-sized files instead of symlinks).
Downloaded branch libtorrent-symlinks, then built on macosx 10.13.6:
bjam link=static variant=release toolset=darwin cxxflags="-std=c++11"

@arvidn
Copy link
Owner

arvidn commented Mar 4, 2019

did you try running test_torrent? Specifically, symlinks_restore.
That test passes for me. could you verify that TORRENT_HAS_SYMLINK is set to 1 for you?

@AlexanderVitiuk
Copy link
Author

symlinks_restore test passed, but for some reason if i use compiled library libtorrent.a in my c++ project and downloading data of symlink2.torrent, it makes zero-sized files instead of symlinks...
Xcode full rebuild made.
TORRENT_HAS_SYMLINK for me == 1
what can be wrong?..

@arvidn
Copy link
Owner

arvidn commented Mar 5, 2019

make sure you rebuild everything that needs rebuilding and that you're linking against the new build of the library

@arvidn
Copy link
Owner

arvidn commented Mar 5, 2019

is there a chance you may be picking up an "installed" copy of libtorrent?

@AlexanderVitiuk
Copy link
Author

Finally i figured out what happened:
I added to xcode project link to libtorren.a in build path.
But linker for some reason used libtorrent.a from usr/local/lib folder.
Now it seems works fine for me.
Thanks for help!)

@AlexanderVitiuk
Copy link
Author

Maybe it is off top, but i have question: is it possible to get sha1 piece hashes from create_torrent class after lt::set_piece_hashes(...), to use them for calculate file hashes for create_torrent.set_file_hash(...)?
We used -f key of make_torrent tool to save file hashes for some sync reason, but since 1.0+ it is removed...

@arvidn
Copy link
Owner

arvidn commented Mar 5, 2019

you cannot calculate file hashes from the piece hashes. The feature to calculate file hashes and piece hashes in a single pass got quite complicated when making the disk I/O threaded, that's why I removed it (as it seemed like a less useful feature than better disk performance).

I'm afraid you'll have to calculate file hashes by hand.

Alternatively, you can try out the v2 branch (which still has some ways to go before it's ready to be merged into `master). It implements support for the bittorrent version 2, which uses per-file merkle hash trees.

@arvidn
Copy link
Owner

arvidn commented Mar 5, 2019

There's actually some more important things that need fixing for this patch. There currently aren't any checks to make sure symlink targets are files from within the torrent. Also, symlinks are not relative to the torrent root right now, as specified in the BEP.

@AlexanderVitiuk
Copy link
Author

AlexanderVitiuk commented Mar 11, 2019

It seems now symlinks can be created before target object is downloaded. In this case, if terminate downloading process and shutdown session just after symlink created, and then start session again from checking/downloading, it is failed when tried to create this symlink again.
After that it is not possible to download this torrent in regular way.

Got such alerts:
FILE_ERROR: Contents symlink (....../SDL2.framework/Resources) error: File exists
TORRENT_ERROR: Contents ERROR: (17 File exists) ....../SDL2.framework/Resources

SDL2.framework/Resources - symlink to folder inside ....../SDL2.framework/ which is not downloaded yet.

@arvidn
Copy link
Owner

arvidn commented Mar 14, 2019

@AlexanderVitiuk could you please test this patch? #3709

It attempts to fix the issue you mention, where the symbolic link already exists. However, perhaps more importantly, it also validates the symlink targets in the torrent file and adds some stricter requirements on them. The BEP47 requires link targets paths to be rooted in torrent root. I allow relative paths for backwards compatibility for now.

@AlexanderVitiuk
Copy link
Author

Downloaded/built branch libtorrent-symlink-fixes.
Made full XCode project rebuild,
Checked: used appropriate libtorrent.a and includes.
Still have described issue (symlink created before target folder downloaded).
On next app launch i got again:
FILE_ERROR: Contents symlink (...../SDL2.framework/Resources) error: File exists
TORRENT_ERROR: Contents ERROR: (17 File exists) ...../SDL2.framework/Resources

Maybe it is important:

  • i tested on torrent-file, made by previous version of library (branch 1.2)
  • failed symlink linked to folder

@arvidn
Copy link
Owner

arvidn commented Mar 14, 2019

Could you try downloading it fresh? That error suggests that the symlink exists, but has a different target than what's trying to be written to it. With this patch, all targets are relative to the link itself, previously the symlink target was not validated by just written verbatim.

This also means that symlink targets must point to another file in the torrent right now (or to a directory). If it doesn't, the symlink is made to point to itself. If this happens, it's possible the existing symlink also mismatches the one trying to be created.

@AlexanderVitiuk
Copy link
Author

I downloaded to fresh folder, but used old .torrent file.
Should i re-create .torrent file using newly compiled make_torrent tool?

@arvidn
Copy link
Owner

arvidn commented Mar 14, 2019

does a newly built dump_torrent print your old .torrent file correctly? (specifically the link target)

@AlexanderVitiuk
Copy link
Author

It seems newly build dump_torrent shows it correct:
...
Contents/Frameworks/SDL2.framework/Resources -> Contents/Frameworks/SDL2.framework/Versions/A/Resources
Contents/Frameworks/SDL2.framework/SDL2 -> Contents/Frameworks/SDL2.framework/Versions/A/SDL2
Contents/Frameworks/SDL2.framework/Versions/Current -> Contents/Frameworks/SDL2.framework/Versions/A
Contents/Frameworks/SDL2_image.framework/Frameworks -> Contents/Frameworks/SDL2_image.framework/Frameworks
Contents/Frameworks/SDL2_image.framework/Resources -> Contents/Frameworks/SDL2_image.framework/Versions/A/Resources
Contents/Frameworks/SDL2_image.framework/SDL2_image -> Contents/Frameworks/SDL2_image.framework/Versions/A/SDL2_image

...

@arvidn
Copy link
Owner

arvidn commented Mar 15, 2019

do you get the error you mentioned when downloading to a fresh (empty) download directory?
could you set a break point on the symlink() call to see what it's trying to set the target to? and since it fails, it should call readlink() right after, can you take a look at what the link actually is on disk?

@AlexanderVitiuk
Copy link
Author

Yes, this error when downloading into fresh empty folder.
I unable to debug libtorrent sources, but you can easily reproduce it by yourself using your client_test tool, it also fails.
Just run
./client_test tt2macosx.torrent
wait till download about 1.7%, then stop it (i used ctrl+c).
Then launch again - i got error "File exists"
(zipped torrent-file attached)
tt2macosx.zip

@arvidn
Copy link
Owner

arvidn commented Mar 15, 2019

thanks, fixed! Please try again

@AlexanderVitiuk
Copy link
Author

Now it is do not create any symlink when downloading...

Also got 1 new warning during library compilation:
src/storage.cpp:343:54: warning: implicit conversion changes signedness: 'const long' to 'boost::basic_string_view::size_type' (aka 'unsigned long') [-Wsign-conversion]
if (ret <= 0 || target != string_view(buffer, ret))

@arvidn
Copy link
Owner

arvidn commented Mar 15, 2019

ok, third time's the charm! I've also fixed the unit test for lexically_relative() and I think I fixed that warning you got.

@AlexanderVitiuk
Copy link
Author

Now it seems works fine for me, thanks for fast fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants