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

Fix outgoing interfaces not getting assigned in case addr is not null. #12430

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

rwasef1830
Copy link
Contributor

Fixes #12421

@FranciscoPombal FranciscoPombal added the Network Issues related to network connectivity label Apr 5, 2020
@FranciscoPombal
Copy link
Member

Can you confirm you get the expected behavior (i.e. the same as client_test) with this change?

@rwasef1830
Copy link
Contributor Author

@FranciscoPombal Yes I can confirm that the problem is solved. Now it is making outgoing connections only from the IP I selected in options. It works correctly whether I choose the IP only, or whether I choose both IP and interface.

@rwasef1830 rwasef1830 force-pushed the add-outgoing-interfaces branch 2 times, most recently from fc7f3ed to 53affcb Compare April 6, 2020 11:53
@rwasef1830
Copy link
Contributor Author

@Chocobo1 done.

Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit messages should be formatted according to the project Coding Guidelines.

@rwasef1830
Copy link
Contributor Author

rwasef1830 commented Apr 6, 2020

@glassez better ? apologies, I'm new to this repo.

@glassez
Copy link
Member

glassez commented Apr 6, 2020

@glassez better ? apologies, I'm new to this repo.

Don't think so... I suppose "in primary branch" refers to implementation details (i.e. to "if-else"). It confuses when reading it itself. But when you read the code it becomes meaningless since code clearly shows it.
Just use your original message but w/o extra detail: "Fix outgoing interface is not getting assigned". You can describe the problem in message body if it is needed.

Assignment was missing in main branch of condition statement.
Closes qbittorrent#12421
@rwasef1830
Copy link
Contributor Author

@glassez ok, is it clearer now ? :-)

@Chocobo1 Chocobo1 merged commit f58f425 into qbittorrent:master Apr 7, 2020
@Chocobo1
Copy link
Member

Chocobo1 commented Apr 7, 2020

@rwasef1830
Thank you!

@Chocobo1
Copy link
Member

Chocobo1 commented Apr 12, 2020

@rwasef1830
I was testing on qbt master branch with libtorrent RC_1_2 latest and I just found out downloading doesn't work anymore. I can see many peers but they all come and go. I reverted this patch and the problem is gone, care to take a look?
It is important that the issue will only come up when "Any interface" option is selected in advanced settings, specifying an interface such as "eth0" won't see the symptom. And "any addresses" is selected during my testing, not sure if it is related.

@rwasef1830
Copy link
Contributor Author

I will investigate. Maybe a special case is needed for handling "any interface" + "any address" when handling outgoing connections, it's possible that a blank or empty or invalid address is being set or something. I will investigate.

@rwasef1830
Copy link
Contributor Author

@Chocobo1 ok the problem is that when selecting "any interface" "any address", outgoing interfaces is set to 0.0.0.0 and :: and that makes all outgoing connections fail (maybe because not valid IP address). Solution is to skip setting outgoing interface when it is equal 0.0.0.0 or [::]. I will make a new pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Network Issues related to network connectivity
Projects
None yet
4 participants