jpdillingham / Soulseek.NET

A .NET Standard client library for the Soulseek network.
GNU General Public License v3.0
173 stars 24 forks source link

Allow binding to a specific IP address #780

Closed J-C-3 closed 9 months ago

J-C-3 commented 1 year ago

Soulseek.NET & slskd are great!

With my existing p2p solutions, I prefer to bind my client to ensure all my traffic is going out of my VPN connection instead of my local connection.

I think it would be great if Soulseek.NET would support binding to a specific IP instead of 0.0.0.0/0 .

I've taken some time today and made some minor modifications to both Soulseek.NET and slskd to support this.

It needs some tests to be adjusted to support it, working through some bugs with live reloading of the config, and some refinement, but if this is of interest to others I would happily open a draft PR and continue.

I've never written in C# before, but if there's interest I'll happily continue onward and open a draft PR.

Thanks all!

jpdillingham commented 1 year ago

Thanks for the suggestion!

I think to get the desired effect, both the Listener and Connection classes need to be updated. I'll put some thought into the best place to put this option.

If you have a branch I'd love to take a look!

J-C-3 commented 1 year ago

Of course, here’s the commit:

https://github.com/J-C-3/Soulseek.NET/commit/d865965f9ffc8a06e8412e4e53c8cc2bcf7b16eb

jpdillingham commented 1 year ago

Thanks, I'll take a look, but probably not for a week or so due to a personal backlog.

J-C-3 commented 1 year ago

No rush at all! Just poking at it a bit for right now and it's not even close to a priority.

As I was poking at it a bit today I got the hot reload working, whether it should be done the way I do I'm not 100% sure at the moment. There is an issue right now where if I change the IP alone live, I get an Address already in use exception.

I was attempting to allow the socket for the listener for address reuse, but was having difficulty accessing the TcpListener.Server through the Listener interface.

I squashed it down to the other commit:

https://github.com/J-C-3/Soulseek.NET/commit/d865965f9ffc8a06e8412e4e53c8cc2bcf7b16eb

jpdillingham commented 10 months ago

Completely forgot about this until I came in today to fix another issue! I created a branch and cherry-picked your squashed commit, I'll do some testing and get this going probably next month when I'm off for the holidays.

J-C-3 commented 10 months ago

Appreciate it, @jpdillingham !

No rush or anything! I appreciate all you’ve done on this project.

I’m no dot net dev or anything, so I’m sure you’ll be able to implement it better than I did.

jpdillingham commented 9 months ago

@J-C-3 this is released in 6.2.0, I'll work on getting it into slskd soon.

You did well, .NET dev or not! I just adjusted a few nitpicky things and had to lose most of the tests because binding was limited to 0.0.0.0 and 127.0.0.1 across environments. I did some functional testing with the app running and am reasonably certain it works, but the true test will come later.

Thanks again for the suggestion and implementation!

jpdillingham commented 8 months ago

@J-C-3 one last ping to close the loop! This capability was added to slskd in 0.19.2

Thanks again for the suggestion and most of the implementation!

J-C-3 commented 8 months ago

@jpdillingham appreciate the feature add! Thanks again for everything!