Yortw / RSSDP

Really Simple Service Discovery Protocol - a 100% .Net implementation of the SSDP protocol for publishing custom/basic devices, and discovering all device types on a network.
http://yortw.github.io/RSSDP/
MIT License
282 stars 67 forks source link

Simple UDP discovery code works but RSSDP example does not #38

Closed waynebloss closed 6 years ago

waynebloss commented 7 years ago

I have a bunch of devices on my network that publish about 25 services with SSDP. Devices include a Synology DiskStation and a couple of Samsung TVs among others.

When I tried to run the RSSDP example (commands: A, R and B) to search for All, Root or Basic devices - it doesn't find anything. I used command L to listen before searching. I also attempted to change the example to use a specific local address as noted in your solution here, but that did not work either:

Change to line 105 of RSSDP/src/Main/Rssdp.Samples/Program.cs:

_BroadcastListener = new SsdpDeviceLocator(
    new Rssdp.Infrastructure.SsdpCommunicationsServer(
        new Rssdp.SocketFactory("192.168.1.99")
    ));

However, the following UDP code for SSDP discovery from this stackoverflow post works after I applied the advice from all three answers... My final code is shown below and when I run it, I get about 240 lines of console output showing the responses from all my devices...

IPAddress localNetwork = Dns.GetHostAddresses(Environment.GetEnvironmentVariable("COMPUTERNAME")).Where(ia => (ia.AddressFamily == AddressFamily.InterNetwork)).First();
IPEndPoint LocalEndPoint = new IPEndPoint(localNetwork, 60000);
IPEndPoint MulticastEndPoint = new IPEndPoint(IPAddress.Parse("239.255.255.250"), 1900);
//IPEndPoint MulticastEndPoint = new IPEndPoint(IPAddress.Parse("192.168.1.255"), 1900);

Socket UdpSocket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);

//UdpSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
UdpSocket.Bind(LocalEndPoint);
//UdpSocket.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.AddMembership, new MulticastOption(MulticastEndPoint.Address, IPAddress.Any));
//UdpSocket.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.MulticastTimeToLive, 2);
//UdpSocket.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.MulticastLoopback, true);

Console.WriteLine("UDP-Socket setup done...\r\n");

string SearchString = "M-SEARCH * HTTP/1.1\r\nHOST:239.255.255.250:1900\r\nMAN:\"ssdp:discover\"\r\nST:ssdp:all\r\nMX:3\r\n\r\n";

UdpSocket.SendTo(Encoding.UTF8.GetBytes(SearchString), SocketFlags.None, MulticastEndPoint);

Console.WriteLine("M-Search sent...\r\n");

byte[] ReceiveBuffer = new byte[64000];

int ReceivedBytes = 0;

while (true)
{
    if (UdpSocket.Available > 0)
    {
        ReceivedBytes = UdpSocket.Receive(ReceiveBuffer, SocketFlags.None);

        if (ReceivedBytes > 0)
        {
            Console.WriteLine(Encoding.UTF8.GetString(ReceiveBuffer, 0, ReceivedBytes));
        }
    }
}
Yortw commented 7 years ago

I'm afraid I don't know what the problem is. I've just run the RSSDP sample on my home network to test, and it works fine here (sample of the first few devices found shown below, but like you I have many devices and 100's of lines of output). It finds my Sony TV, Synology NAS, home theatre, sonos system etc all fine, without any special mods or setup. I just ran the exe from the last time it was built. I'm also using this in a commercial app at work where we both publish the device (server) and locate it from the clients (mobile devices). That was working last week.

image

The sample code you posted looks correct, and is basically what RSSDP does albeit with more features/options/error handling etc. It sends a search request for all devices to the multicast address used by SSDP and then receives from the same socket that broadcast message was sent on. That is the basic pattern for implementing SSDP, though it obviously doesn't handle parsing the responses, different search request types, any form of publishing, notification listening, error handling etc.

Given it works here, and I can't debug on your network, I'm not sure how to help. You could try debugging the sample (with project references to the RSSDP projects) to see if you can determine where it's going wrong.

You could also try publishing a test device with the sample app, and running another copy to search to see if it finds that, though I'm not sure why that would behave different to the real devices, unless there is a network or firewall issue blocking the RSSDP requests and not your sample.

You could also try the Intel Device Spy UPNP app on another machine on the network to see if you can see the broadcast search request from the RSSDP sample. If you can't see it then presumably the packet is blocked by a firewall or being broadcast on the wrong network adapter.

Also, are you running on the .Net framework on Windows, or are you running on Net Core under Linux/OSX, or Xamarin on iOS/Android? The test above was only done on Windows on the desktop framework, though the other platforms should work. I have previously tested on Android/iOS and others have tested on Linux/OSX.

Let me know if there's any more detail, or anything I can do to help, but given the fault is not reproducible I sort of suspect it's environmental.

waynebloss commented 7 years ago

Thanks for your response. One point of note: If I run 2 instances of the sample, I can see services published by the other instance of the sample as you can see below.

But those are the only services that I see. This leads me to believe that maybe somehow I'm hitting that problem where the wrong local IP Address is being used.

2016-11-05 21_38_12-c__windows_system32_cmd exe

waynebloss commented 7 years ago

No problem - I'll be able to figure it out since I have your code. I really need the publishing side of things to work in C# anyway, not the discovery.

Also, final note: The listener is working fine. If I leave that running I see "Alive Broadcast" from the Synology device.

Yortw commented 7 years ago

Hmm, odd that the notifications work but the search doesn't. They are a bit different in that the notifications are published periodically on the broadcast address, whereas the search responses are sent to the unicast port the search request was sent from, but I don't really know why one would work and the other not.

Given you can see the devices from a second instance but not outside the same PC, it does sound like possibly the wrong network adapter/address is being used, or possibly there is a firewall issue blocking the unicast responses and not the broadcast stuff.

Defintely let me know if I can be of more help or you find a problem.

danilevich commented 7 years ago

This solution? Works for me (android phone): ... Socket UdpSocket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp); UdpSocket.ReceiveTimeout=5000; ... int repeat=0; while (repeat++<5/*udpSocket.Available > 0/) { ReceivedBytes = UdpSocket.Receive(receiveBuffer,64000, SocketFlags.None); if (ReceivedBytes > 0) { receivedString += System.Text.Encoding.UTF8.GetString(ReceivedBytes/_, 0, receivedBytes*/)+"\n"; break; } }

maybe repeat it isn't necessary or timeout to change

LukePulverenti commented 7 years ago

Try changing these messages from static strings to something that you construct manually so that you can explicitly insert \r\n in between each line:

https://github.com/Yortw/RSSDP/blob/master/src/Main/RSSDP.Portable/SsdpDevicePublisherBase.cs#L43

Along with other the messages in the locator base class as well.

Yortw commented 7 years ago

@waynebloss Have you managed to make and progress on getting this to work on your end? @LukePulverenti Do you have any reason to suspect the static strings in RSSDP are somehow missing the correct \r\n sequences at runtime? It's working for me but if there's something broken with that design I could change it.

LukePulverenti commented 7 years ago

I suppose a unit test is the best way to prove it, but make sure to run that test on all supported platforms.

I've had to add additional workarounds that we've accumulated over the years in order to support more devices that don't follow spec and/or have bugs of their own. The reason I have not submitted pull requests is because I do not have exact data to tell you which devices the changes apply to.

It is my own fault for not capturing that information when originally making the code change to our own implementation 2-3 years ago. We recently switched to RSSDP because I liked the unit test coverage, but when users started reporting things like devices not showing up, I knew immediately I had to go dig up our old workarounds and add them to RSSDP.

Here is an example: https://github.com/Yortw/RSSDP/blob/master/src/Main/RSSDP.Portable/SsdpDevicePublisherBase.cs#L699-L702

While that is correct according to spec, it will cause some devices to be missed. Similarly things like this, where the request is aborted as opposed to creating a default value for mx:

https://github.com/Yortw/RSSDP/blob/master/src/Main/RSSDP.Portable/SsdpDevicePublisherBase.cs#L291-L300

Also AliveNotificationMessageFormat is not upper-casing Date, which isn't supposed to matter, but it might matter if a device is looking for the header in a case-sensitive manner.

I've been able to restore our features for most of our users, but there is still a small group that remains affected with devices not being found: https://emby.media/community/index.php?/topic/40243-bug-dlna-play-to-broken/

Once I've chased that down I will update on what the resolution was.

Yortw commented 7 years ago

Thanks for the info Luke. I'd really like to help get these problems sorted. I would prefer RSSDP worked in reality than in theory, so handling poorly behaved devices is something I'm prepared to do (at least in some instances, it might depend on how big a hack is needed). The problem seems to be I don't have any poorly behaved devices and everything works for me.

On that note, any information you can provide to help me sort these issues out, or any pull requests you can submit, would be greatly appreciated. Handling a missing MX/MAN header seems like something I could do. Please let me know if there's anything else I can look at.

However this doesn't seem to explain @waynebloss's original problem, as the RSSDP publish and locator should both be well behaved (or at least match in any poor behaviour), and they couldn't find each other on his network, so his problem seems to be different.

I'm still confused about the line endings issue. I already had to fix one bug previously where it didn't work under mono because somewhere I did use StringBuilder, and it swapped to the Linux line ending by default, which then wasn't compatible with the spec and failed in a lot of places. I believe I still used StringBuilder but had to stop using AppendLine and insist on manually adding "\r\n", so just switching to SB isn't neccesarily going to help. Also, given those messages are hard coded with "\r\n" I don't see how there could be a problem (unless the source file has changed, but then it should break for everybody). Do you have any further explanation/info on why you suspect the line endings in this case?

Yortw commented 7 years ago

PS: I had no idea RSSDP was being used in anything Emby related, so that's pretty cool to hear. Unfortunately 'running the tests on all platforms' is also a problem for me, as I really only have Windows devices and even if I had spare hardware etc. I'm not familiar with the other OS'/environments. Support for Mono/.Net Core on Linux etc. has mostly been implemented by others and I rely on them to ensure it works well :(

LukePulverenti commented 7 years ago

Some of our workarounds have also been based on defects in older mono versions, so now that mono is adopting more reference source from Microsoft, it's possible that the line endings are a non-issue. This all comes back to me not documenting the specifics of each workaround, which essentially forces me to keep them around indefinitely until new information comes to light.

At this point there are only a couple major differences I can think of between what we have now with RSSDP, and our previous homegrown implementation. The first one is that we previously used Ascii encoding for all responses.

The second one is that we had a listener open on each network address in addition to IpAddress.Any. It's convenient to use IpAddress.Any but I might have to try that old approach if I can't think of anything else.

I have a situation on a Windows 10 box that might help. After some random period of time (e.g. several days), all upnp connections will be lost, both in terms of discovering other devices and devices discovering us. Restarting the application does not help, but restarting the network adapter does resolve it - until it eventually happens again. My hope is that the next time this happens, if I can resolve connectivity without restarting the network adapter, then hopefully that will resolve the discovery issues.

Yortw commented 7 years ago

Thanks.

The issue with having to restart the network adapter sounds like it might be a driver level issue, or possibly something .Net itself? It sort of sounds like a socket dying and being left open indefintely or something.

Is the ASCII encoding a problem? I don't recall but assume I'm using UTF8. My understanding is that should work the same as ASCII if only ASCII characters are sent, and should cope better with anything uses UTF8, so I wouldn't expect it to cause a problem?

The IPAddress.Any thing is annoying. If I'd known about the issues when I'd started I probably would have done it differently, but it's a bit hard now without a redesign. My suggestion so far has been to do what you said but at a higher level, i.e instead of RSSDP listening/publishing to multiple addresses the app should create a separate locator/publisher for each address. Especially true in my commercial app where there are often multiple networks but we really do only want to publish/search on one (though knowing which one without user config is problematic).

Yortw commented 7 years ago

I checked your RSSDP fork to see if you'd made any commits you hadn't submitted as pull requests, but I didn't see any. Did I miss them?

LukePulverenti commented 7 years ago

I doubt the Ascii encoding is the problem, I'm only mentioning it because it's one of the last few differences I can think of.

I guess that could be the socket dying and being left open, I was also wondering if it might be Windows ssdp trapping the messages (which your code mentions).

I actually ended up embedding RSSDP into our dev branch:

https://github.com/MediaBrowser/Emby/tree/dev/RSSDP

The main reason is that as these issues are being reported to us it's just been easier to have it that way. Also, we now have the server running as a .NET core app, and the easiest way to achieve dual compatibility with .NET and .NET core has been to just make everything PCL. However, I would prefer PCL without compromise. The PCL RSSDP pulls in quite a bit of nuget packages for BCL and these can be removed if you're able to target a higher PCL profile (which we are). This in turn also allows removal of TaskEx in favor of framework methods.

LukePulverenti commented 7 years ago

If none of these ideas pan out then i'll review all of the socket options we were setting in our old implementation and hunt down the exact differences. I would really prefer to avoid that because user logs were constantly filled with socket errors in the old implementation.

The main reason for using RSSDP was that it was designed for mobile/low-profile, and our previous implementation was very heavy. Ironically, I was also hoping to avoid having to spend so much time on Upnp learning all of the gotchas, but it's clear to me now there's no escaping it if you want users to be happy.

Yortw commented 7 years ago

Yeah, it's a shame. However I would like to see you keep using RSSDP and have the project improve if possible. While my time for these side projects is limited, I'm happy to help where I can. All the known/reproducible issues that have been reported to me are fixed (I believe), so it's mostly a matter of knowing what to do (or at least what the problems are).

LukePulverenti commented 7 years ago

We're sticking with it. The unit testing that you have is important to me. Once i have figured this out I will let you know what solved it for us.

Yortw commented 7 years ago

Thank you. I will take a look at some of the issues you've raised already when I get time and see what I can do. So I can prioritise, is that going to be any use to your or is your branch too far evolved to take any changes I make back?

LukePulverenti commented 7 years ago

We will definitely incorporate upstream changes. If a full alignment is possible i would love to do that as well, although I think your windows phone 8 requirement means you need all those BCL packages.

One other change I made is a fixed interval for blast messages. Not because there's anything wrong with your approach of cache length + randomization, it's because we have an interval setting in our configuration that we need to respect. Many upnp-related have that sort of setting as well so it's not uncommon.

One thing about the current approach is that the timer has to be changed everytime it fires. This is OK but if you configure the cache length in a way that causes this condition to be hit:

https://github.com/Yortw/RSSDP/blob/master/src/Main/RSSDP.Portable/SsdpDevicePublisherBase.cs#L559

Then it will return and stop sending messages indefinitely until a device is added or removed. If I can recall how I created that situation I will let you know. In the end, I found it simpler just to have a timer fire at an interval so that it doesn't have to be continuously updated.

LukePulverenti commented 7 years ago

For organizational purposes, I'll try to do separate topics for other things i think of, because I realize I've taken you off topic (apologies).

Yortw commented 7 years ago

Was thinking about creating some new issues myself. Thanks for that extra info. I believe the cache time + randomisation was a suggestion of the spec, to avoid all broadcasts overlapping or something. I will have to recheck the spec. I don't (off the top of my head) see any problem with allowing a configuration option for a fixed interval broadcast if the client chooses, and we can do the random thing by default I guess.

dotorg commented 7 years ago

In case its useful, I saw the same issue on my workstation, and it was two problems, both of which had to be solved in order to get the search to work. As with the original report, if I happened to catch a broadcast notification about a device, I'd see it, but the direct search results, I got no data at all.

As it turns out, it was two things. First, the wrong IP address was being selected by the listener. I have HyperV installed, a couple of inactive VPN endpoints and both Ethernet and WiFi devices on this desktop, The system was picking up an autoconfigured IP address on one of the HyperV networks instead of the "public" one that exists on my local network.

Secondly, it appears that the Windows SSDP service sitting on port 1900 was picking up the responses, even when I did hard code the correct IP address. A different response port was needed.

Creating an instance like this: var _DeviceLocator = new SsdpDeviceLocator(new SsdpCommunicationsServer(new SocketFactory(GetLocalIpAddress()), 19000));

That worked fine, by passing a different port (19000 in this case, instead of 1900). GetLocalIpAddress looks like this (ripped from the stackoverflow linked above -- but of all the "examples" in there, it was the only one that worked with the HyperV crap on this system):

`public string GetLocalIpAddress() { UnicastIPAddressInformation mostSuitableIp = null;

        var networkInterfaces = NetworkInterface.GetAllNetworkInterfaces();

        foreach (var network in networkInterfaces)
        {
            if (network.OperationalStatus != OperationalStatus.Up)
                continue;

            var properties = network.GetIPProperties();

            if (properties.GatewayAddresses.Count == 0)
                continue;

            foreach (var address in properties.UnicastAddresses)
            {
                if (address.Address.AddressFamily != AddressFamily.InterNetwork)
                    continue;

                if (IPAddress.IsLoopback(address.Address))
                    continue;

                if (!address.IsDnsEligible)
                {
                    if (mostSuitableIp == null)
                        mostSuitableIp = address;
                    continue;
                }

                // The best IP is the IP got from DHCP server
                if (address.PrefixOrigin != PrefixOrigin.Dhcp)
                {
                    if (mostSuitableIp == null || !mostSuitableIp.IsDnsEligible)
                        mostSuitableIp = address;
                    continue;
                }

                return address.Address.ToString();
            }
        }

        return mostSuitableIp != null
            ? mostSuitableIp.Address.ToString()
            : "";
    }`

In case that helps any... I'm not sure it actually requires any changes in the library, but a comment that you need to set the IP and socket manually in some cases in the docs would probably help others who run into it.

Yortw commented 7 years ago

Thanks for the info. The FAQ sort of covers this already, but perhaps could be more explicit so I'll look at updating it when I get chance. Unfortunately @waynebloss has said he already tried assigning a specific IP and port.

I'm also not sure why you had to specify a non-1900 port, as the default code already uses a port that isn't 1900 specifically to avoid issues with the SSDP service on Windows. Specifically, it should be passing port 0 by default to the .Net functions, and .Net should be choosing a random port.

The suggested code for guessing the best adapter is very similar to what's in the library already, but it does have some extra checks which might be useful to implement. I'll do some testing when I get a chance and if they seem good I'll update RSSDP to do the same.

I also have Hyper V, as well as two physical NICs installed and a bunch of virtual adapters for VPNS etc, and yet the current code choose the 'correct' adapter for me. I suspect that any attempt to 'guess' the correct adapter will be wrong on some % of systems some % of the time, so being specific if you can is probably best.

Yortw commented 7 years ago

Ah. I thought I had code similar to this in the library, but it turns out it's in one of the other projects I built that uses RSSDP rather than RSSDP itself. Will look and see what to do about that.

Yortw commented 6 years ago

Closing this thread as no response from OP after last updates/queries. Please re-open if there is new information or you are still experiencing the problem.