OK the D-LInk stuff seems to parse fine with the current SVN version.

As for twonky - I realise that it;'s a media server and the UPnp supported isn't for AV servers - but the question is why does the plugin attempt to add it to the routers description and is there some means for us to ignore it?

Well simply put UPnP goes like this :
- KT broadcasts a packet to discover all InternetGatewayDevices on the network
- A UPnP device which thinks this applies to him sends back a response
- KT will download the xml description of all UPnP devices which have responded and it will try to parse that

I don't know why twonky responds, it is not an InternetGatewayDevice.

I've played around a bit, and read up on UPNP device discover at http://www.upnp.org/download/UPnPDA10_20000613.htm

Must say, am finding this good fun...

Anyway,
After checking, I don't think Twonky is actually responding to the dicovery search request that KT sends out - just seems Twonky advertises itself many times and the UPNP plugin finds it..

I think what's happening is the following:
1, KT broadcasts a search discovery
2, KT then listens for any responses
3. Twonky or other upnp sends a discovery advertise broadcast
4. KT detects it and attempts to process.

The problem is that in a UPNP network, any device can send an advertise broadcast. KT on receiving any response should parse it to ensure
a) it's in response to it's search broadcast
and
b) it is actually an Internet Gateway that is responding.

What makes me think this...
- well my router responds to the discovery search broadcast with a valid search target (ST:) field. This is correct behaviour in response to a discovery search. Twonky on the other hand is sending no ST: field, but is sending a notification type (NT:) field which is correct for discovery advertisement broadcasts.

Simple solution...
while parsing the response for location and server - parse it for a line which starts with ST and contains "InternetGatewayDevice". e.g.

Code: Select all

         else if (line.startsWith("Server") || line.startsWith("server") || line.startsWith("SERVER"))
         {
            server = line.mid(line.find(':') + 1).stripWhiteSpace();
            if (server.length() == 0)
               return 0;
            Out() << "Server : " << server << endl;
         }
         else if (line.startsWith("ST:") && line.contains("InternetGatewayDevice"))
         {
            validDevice = true;
         }


Only if this check is valid should KT add the server to the routers - otherwise discard it.

Code: Select all

      if (!validDevice)
         return 0;

      if (routers.contains(server))
         return 0;


To be honest - this still lets a lot of stuff get through to the log - i.e. KT still parses invalid devices. A more logical, but time consuming, thing to do is check for the response up front, and only if it's a valid device parse the response fully, e.g.

Code: Select all

   UPnPRouter* UPnPMCastSocket::parseResponse(const QByteArray & arr)
   {
      QStringList lines = QStringList::split("\r\n",QString(arr),false);
      QString server;
      KURL location;
      
      // first read first line and see if contains a HTTP 200 OK message
      QString line = lines.first();
      if (!line.contains("HTTP") && !line.contains("200"))
      {
         return 0;
      }
      
      // quick check that the response being parsed is valid
      bool validDevice = false;
      for (Uint32 idx = 0; idx < lines.count() ; idx ++)
      {
         line = lines[idx];
         if (line.contains("ST:") && line.contains("InternetGatewayDevice"))
         {
            Out() << "Valid Internet Gateway Device has responded, parsing response...." << endl;
            validDevice = true;
         }
      }
      if (!validDevice)
         return 0;

      // read all lines and try to find the server and location fields
      for (Uint32 i = 1;i < lines.count();i++)
      {
         line = lines[i];
         if (line.startsWith("Location") || line.startsWith("LOCATION") || line.startsWith("location"))
         {
            location = line.mid(line.find(':') + 1).stripWhiteSpace();
            if (!location.isValid())
            {
               Out() << "Invalid URL" << endl;
               return 0;
            }
            Out() << "Location : " << location << endl;
         }
         else if (line.startsWith("Server") || line.startsWith("server") || line.startsWith("SERVER"))
         {
            server = line.mid(line.find(':') + 1).stripWhiteSpace();
            if (server.length() == 0)
               return 0;
            Out() << "Server : " << server << endl;
         }
      }
      
      if (routers.contains(server))
         return 0;
      
      // everything OK, make a new UPnPRouter
      return new UPnPRouter(server,location);
   }


By doing this KT enforces itself to check for only responses it requested.

RikD.

Indeed some more checking can never hurt. I'll make the modifications.