-
Notifications
You must be signed in to change notification settings - Fork 65
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
WIP: accept tracker connections #187
base: master
Are you sure you want to change the base?
Conversation
852c04f
to
ad725e2
Compare
a0b0f80
to
dc0797b
Compare
dc0797b
to
f9e6a92
Compare
src/main/java/org/araymond/joal/core/ttorrent/client/ConnectionHandler.java
Show resolved
Hide resolved
selector.select(); | ||
for (SelectionKey key : selector.selectedKeys()) { | ||
if (key.isAcceptable()) { | ||
this.listenChannel.accept(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note no I/O is done here at the moment, we only accept the connection. Any ideas how to proceed, if at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro vision is:
- When a peer connect though bittorrent protocol => send a choke message
But "connect through bittorrent protocol" is easier said than done 😆 .
it's something related to : https://github.com/mpetazzoni/ttorrent/blob/2c8bb62f9f69b74c014358d446554df96ec91c7c/ttorrent-client/src/main/java/com/turn/ttorrent/client/network/HandshakeReceiver.java#L148
Instead of adding peers to whatever store you'll just choke them (maybe close connection?).
This feature require a deep comprehension of the peer section of BitTorrent RFC (and also to investigate the source code of libtorrent and other OpenSource clients to understand how they individualy do their job).
Ye this is going to need quite a bit of research/know-how. |
Two users. :-) I have been waiting for this merge, and updated Docker image, since the opening of this PR. |
Hello there, i've not forgotten about you. But i'm lacking time to review that. I try to give it a look ASAP |
Do you have any idea if and when you'll have the time to look at this, @anthonyraymond? I don't want to rush you, I'm already very grateful for what you did with this project so far, I'm just looking for an ETA. |
Hello, An analogy for this PR is:
|
That is a valid point. Would it be possible to make it optional in config? @laur89 @anthonyraymond |
@anthonyraymond i think it's best to just add a toggle in settings. for me it works great but maybe others can have issues, indeed. can't wait to see this merged and published <3 |
I don't think the config option will come anytime soon either, right? 😔 |
@anthonyraymond can we please merge this as well? been using it since then and it's all good. maybe add a config toggle in the settings just in case? |
@rursache i don't think this is ever going to be added as is. It's sounds a bit dangerous for most people. I'll include a real and complete implementation in the next version (built from scratch), but for now i prefer not to include it as is |
thank you, looking forward to your implementation! |
Note this change is on top of #172 -- merge that beforehand.merged