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

Add the ability to read the announce URL from a file #20

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add the ability to read the announce URL from a file #20

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 16, 2018

Currently the announce URL only can be specified with a CLI argument. This is bad if the URL contains a passkey as it is standard under private trackers. In a shared environment this is highly insecure because under standard conditions everyone can see the CLI arguments from all running processes, so an attacker or an users who knows what he's doing can harvest passkeys from other users. But if the announce URL is read from a file instead nobody can see what the file contains.

@Rudde
Copy link
Collaborator

Rudde commented Jan 17, 2018

Great addition. I see it doesn't support multiple announce url's? Maybe an idea to add as some private trackers use up to (as I've seen) three unique announce URL's to prevent dependency on one.

The changes in main.c also seem irrelevant to your pull request. If they are. Please remove them.

Also it would be nice if you took the files size into consideration when you allocate memory and not always allocate the maximum you allow. It is a lightweight torrent creator.

@ghost
Copy link
Author

ghost commented Jan 17, 2018

@Rudde I've never heard of a private tracker that allows multiple announce URLs, but if you say that one like that exists then I'll add support for it.

Yeah, I'll use the filesize of the file now that's much better.

OK, I can revert the changes and you're right they have nothing to do with that PR, but it's still much more efficient probably to do it like that. Do you have anything against that? Would you accept it in a separate PR?

@Rudde
Copy link
Collaborator

Rudde commented Jan 17, 2018

Yes they exist, and it's also nice to keep it consistent with the original features of announce adding.

Super!

Yes, I can do that, but I would have to test it first. Don't really have anything against it.

@ghost
Copy link
Author

ghost commented Jan 17, 2018

OK, all done. But I've use newline as a separator in announce files as it is more natural and easier to read in files.

@ghost
Copy link
Author

ghost commented Jan 25, 2018

Any update on this?

@Rudde
Copy link
Collaborator

Rudde commented Jan 28, 2018

Hello, I tested this today, but I can't seem to get multipler announce files on new lines to work, it seems to only honor the first one.

@ghost
Copy link
Author

ghost commented Jan 28, 2018

Gotta look into it later.

@ghost
Copy link
Author

ghost commented Jan 29, 2018

Hello, I've tested it and it works for me. How are you creating the torrent? And what are you using as announce file? Can you show me both?

@Rudde
Copy link
Collaborator

Rudde commented Apr 15, 2018

@Hyleus I'm terrible sorry for the late respond, I have overseen the e-mail notification of your latest post.

Content of this: https://gist.github.com/Rudde/b0232e92288e7a3fbe6caf4e91c96786

I had as announce.txt file

use the following to create torrent file

$ ./mktorrent/mktorrent -A announce.txt -p -o from_file_three_announce.torrent file.tar.gz

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

Successfully merging this pull request may close these issues.

1 participant