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

[feature] enable running snapdrop in local network #558

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

Conversation

schlagmichdoch
Copy link
Contributor

@schlagmichdoch schlagmichdoch commented Jan 6, 2023

I wanted to run snapdrop as an instance on a raspberry Pi but devices where not shown to each other.
Devices are grouped by their ip addresses. On local networks all devices have different ip adresses and therefor do not see each other.
As the ip addresses shown to the server are private in that scenario, I added a function isIpPrivate() to differentiate that case and group all devices with private ip addresses together in one room.

The function isIpPrivate() is based on those two stackoverflow discussions as a combination of both:
https://stackoverflow.com/questions/13969655/how-do-you-check-whether-the-given-ip-is-internal-or-not/74891529#74891529
https://stackoverflow.com/questions/35374207/how-to-determine-if-ipv6-address-is-private

…ame room to make it possible to run snapdrop on the local network
@blipk
Copy link

blipk commented Jan 28, 2023

Hey @schlagmichdoch this PR doesn't work for me, looks like it's missing the definition for isIPv4()

 /home/node/app/index.js:202
         if (net.isIPv4(ip)) {
         ^
 
ReferenceError: net is not defined

@schlagmichdoch
Copy link
Contributor Author

Hey @schlagmichdoch this PR doesn't work for me, looks like it's missing the definition for isIPv4()

Hey @blipk Actually, it's the whole package net that is missing as I forgot to commit the import -.-

I just added the following line to this PR which resolves the error:

const net = require('net');

Please try again!

@blipk
Copy link

blipk commented Feb 4, 2023

@schlagmichdoch I just changed the line to:
if (!ip.includes(":")) {

Seemed silly to me to pull in another module for just one function, and I'm pretty sure all ipv6 addresses will have a semicolon, feel free to let me know otherwise.

Also you're still checking the same comparison twice on L192.

I also tried with your change now, and it still doesn't work, not detecting as a peer with my android phone
Perhaps something needs to be changed in the client code, or on the app.

EDIT:
Looks like I have to point the "Base URL" in the android app settings to the scheme, IP and port that my instance is running on, it's working with that change.

It'd be nice if there was some level of auto-discovery, so I could use my home instance at home, and others elsewhere.
I noticed you have forked this to "PairDrop", are you making changes to the app as well? I might give that a go.

@schlagmichdoch
Copy link
Contributor Author

schlagmichdoch commented Feb 4, 2023

@blipk net is built-in to node so it wouldn't matter much but your right, I applied your change. Additionally I moved the prefix removal outside of the ipIsPrivate method.

I'm not sure what you mean with the auto-discovery feature and how this could be implemented. I'm also not sure what the android app is doing. As this is a PWA, there should be no need for an additional app.

The PairDrop fork provides an added pairing functionality to enable sending outside of the local network. There are multiple other changes, lots of stability fixes and ongoing development so feel free to find out more in the Readme, give it a go and add feature requests or issues.

@blipk
Copy link

blipk commented Feb 5, 2023

@schlagmichdoch
I don't use the PWA, there's an app on the F-Droid store, I had to change the setting I mentioned in order for it to work over the local instance.
By auto-discovery I mean it would be good to scan the local network for instances (or have the instance broadcast themselves) and if there's none local then revert to snapdrop.net etc.

@schlagmichdoch
Copy link
Contributor Author

there's an app on the F-Droid store

I'll take a look at that. Not sure whether that is compatible with PairDrop. I'll create an issue.

By auto-discovery I mean it would be good to scan the local network for instances (or have the instance broadcast themselves) and if there's none local then revert to snapdrop.net etc.

Why would you use multiple instances? Just make your private instance publicly available via port forwarding in the routers settings and you can always use your private instance

For any further discussion you should propably create a new issue here or on my repo to keep comments here on topic

@blipk
Copy link

blipk commented Feb 7, 2023

Setting up port forwarding is not so easy for a lot of people, many ISPs are using CGNAT these days and/or only offer IPv6 which is still limited.

I mostly use it to share between my devices at home anyway, just figured I'd mention.

@schlagmichdoch
Copy link
Contributor Author

schlagmichdoch commented Feb 9, 2023

True, but I guess it’s wise to keep the instances separated from each other so front and backend is matching. If you want to use two instances and don’t want to install two PWAs with the Snapdrop for Android app you can change the base url in the settings. Maybe you could create a feature request for quick changing between instances at its repo? 🤷🏽‍♂️

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.

2 participants