Packet Send - Fragmentation?

[zac]

I have a little question here:
Does the SocketTCP make sure that what the client is sending as ONE packet can be received by the server also using SocketTCP as ONE packet? Or do you have to use packet length values? (huge difficulties and potentially dangerous possibilities of programming errors (e.g. allowing Exploitation/Remote Code Injection, we all know Teardrop a.s.o. working with packet fragmentation; demonstrating that it may be a complex topic to handle fragmented packets) may arise - I would request a socket class automatically handling this issue.)

Then, if you have to do so, how do you measure the amount of data that is still left to read? Use Packet::GetDataLength()?
Does Receive() Append() or first Clear() and then Append() (last case would really be crappy)?

Will something like this do the trick (assuming the length value (packet length including the length value itself) is sent first - the whole thing is multithreaded and has to listen to a bool "running" value, so we can't use sf::Selector to do timeouts...):

Code: [Select]


sf::SocketTCP sock; //(this will be properly initialized)
sf::Packet p;
Uint32 length = 0;
sf::Clock clock;
sf::Socket::Status sstate;
sock.SetBlocking(false);

do
{
     sstate = sock.Receive(p);
     if(sstate == Socket::NotReady)
     {
           Sleep(0.05f);
           continue;
     }
     else if(sstate == Socket::Done)
     {
          if(!length) //we do not have a length value now
          {
               if(p.GetDataSize() > 4) //can we read 32 bit?
               {
                    p >> length; //will this decrease the "GetDataSize()" value by 4???????
               }
          }
          else
          {
                if(p.GetDataSize() < length)
                {
                       continue;
                }
                else if(p.GetDataSize() > length) throw 1; /*error - what happens with 2 packets sent with VERY low time gap? With packets send at the same time?*/
                else break; // we have collected all the data and nothing MORE
          }
     }
     else throw 1; //error
}
while(clock.GetElapsedTime() < 5.f); //Timeout



Packets coming fast one after the other could cause problems too - in fact, this should be very likely (would be treated as an error here, but that is some kind of really unwanted behaviour... So this causes even more pain, using serial numbers for each packet or likewise and treating the stuff read from a socket unwillingly... I would like to have a receive function giving me the power to say which amount of data I am requesting AND using sf::Packet. What if I use the Pointer/Length Receive function? Will the received data on the socket NOT fitting in the buffer given STAY there on the top of the socket?

I think this would be important to point out in the tuts...

[bullno1]

I think the network library of SFML just aims to be a socket wrapper. He intended to keep the data "raw", no high-level stuff is involved.

[Laurent]

sf::SocketTCP handles all this stuff very well. Take a look at the implementation if you want more details

[zac]

So I don't have to care about this? Thats great...

[Laurent]

Absolutely

[zac]

Hmmm...
I'm encountering several problems with the SocketTCP class:
In the moment, I am sending data via loopback. The client uses a blocking socket to send 4 byte of data in a sf::Packet. The server does not use a blocking socket, and he can't do so. (Well, this is REALLY strange, because a blocking socket absolutely blocks even if there is no data left, if the client side does not use sf::Packets (for example, if I try to connect to the socket with netcat). A stable server can not make any assumptions about the thingys connecting to it and about the amount of data sent, so it is not possible to use a blocking socket for me here.)

Now, the problem is, that if I try to receive the 4-byte-packet sent by the client, this sometimes (but rarely) works, but mostly it refuses to work. I am using a sf::Selector to announce me if any sockets are ready to read, but if I try to read from them, the length of the packet is 0.
The whole thing works like the tutorial:

1. Set up a listener, add it to a selector, and wait on this selector.
2. There are some sockets ready to read. If the socket is the listener, accept the connection, and, if successfull, add the new socket to the selector.
3. Else, try to read from the socket. If there is no data (packet length == 0), put the socket into a map together with a Clock to give the socket 1 second for the data of the data to arrive (assuming that the selector will alert me if this happens).
4. If there is data, process it.

... Step 4 is never reached. The length of the packet stays 0, even if the selector thinks there should be data to read.

And no, the tutorial also does not work. The client immediately sees "Press enter to exit...". But the connection is established (the server displays "Client connected ! (127.0.0.1)" and the loopback interface is working.

[Laurent]

Have you tried the SVN version? Several fixes have been made since SFML 1.3.

[zac]

I think I am using the SVN version, but I will try to update all files.

[zac]

Nope, this is not working.
The packets are not coming through, but it now works far more often than before - maybe 50% of the cases.

It just seems like a simple client like this:

Code: [Select]


        SocketTCP ssock;
if(ssock.Connect(4223,"127.0.0.1",1.f))
{
Packet p;
p << (Uint32)0x80A1DA12;
ssock.Send(p);
p.Clear();
ssock.Receive(p);
cout << p.GetDataSize() << endl;
ssock.Close();
}
return 0;

is sending packets with a length of 0 sometimes.

Well... I checked this with wireshark. The packets are not sended empty, but with wrong TCP checksum... this is most probably a bug, isn't it? There are 2 data packets send from client to server, the first one I think contains the length (0x4), and the second one with 4 bytes of data. But... they both have the same (both times wrong) checksum... uh? Or is a checksum of 0xfe2c normal and used always by SFML ?!
Hmm, well... somehow this is true also for connections that are working... strange.

[Laurent]

The checksum is added at a lower-level layer (probably TCP), and has nothing to do with SFML.

[zac]

Yes, it is the TCP checksum... but I don't think that this is the bug (because it seems to be wrong even for packets sent/received correctly) ... the data is sent and seems to be valid... but somehow... the server does not receive it. I am giving a timeout of 1.5 seconds to receive the packets (I am using non-blocking sockets to receive)...
And using the loopback... and still, this 4-byte-packet is not received and Receive returns with NotReady...

[Laurent]

Can you provide a complete and minimal sample code which demonstrates this bug ?

[zac]

Well... I think I can try.
Trying to keep it short I omit some error checks et cetera...
Server (C&P should now be possible):

Code: [Select]



#include <iostream>
#include <vector>
#include <string>
#include <SFML/System.hpp>
#include <SFML/Network.hpp>

using namespace sf;
using namespace std;

struct connection
{
       sf::SocketTCP socket;
       sf::Clock timeout;
       sf::IPAddress ip;
       sf::Packet data;
       bool operator==(const connection& other) const;
};

class ConnectionManager
{
     private:
          sf::Selector<sf::SocketTCP> sockets;
          std::vector<connection> connections;
          sf::SocketTCP listener;
      public:
          ConnectionManager();
          ~ConnectionManager();
          void run();
};

bool connection::operator==(const connection& other) const
{
        return (socket == other.socket && ip == other.ip);
}

ConnectionManager::ConnectionManager()
{
       listener.Listen(4223);
       listener.SetBlocking(false);
       sockets.Add(listener);
}

ConnectionManager::~ConnectionManager()
{
       for(int i = 0; i < connections.size(); i++)
             if(connections[i].socket.IsValid()) connections[i].socket.Close();
       listener.Close();
}

void ConnectionManager::run()
{
while(1)
{
       for(vector<connection>::iterator i = connections.begin(); i < connections.end(); i++)
       {
              if(!(*i).socket.IsValid())
              {
                    i = connections.erase(i); i--;
              }
              if((*i).timeout.GetElapsedTime() > 1.5)
              {
                    i->socket.Close();
                    i = connections.erase(i); i--;
              }
       }
       unsigned int ready = sockets.Wait(0.005f);
       for(unsigned int i = 0; i < ready; i++)
       {
               SocketTCP sock = sockets.GetSocketReady(i);
               if(sock == listener)
               {
                      SocketTCP newcon;
                      IPAddress ip;
                      if(listener.Accept(newcon,&ip) == Socket::Done)
                      {
                             sockets.Add(newcon);
                             connection c;
                             c.socket = newcon;
                             c.socket.SetBlocking(false);
                             c.ip = ip;
                             c.timeout = Clock();
                             connections.push_back(c);
                      }
               }
       }
  for(vector<connection>::iterator i = connections.begin(); i < connections.end(); i++)
  {
          if(i->socket.IsValid())
          {
                  Socket::Status state;
                  state = i->socket.Receive(i->data);
                  if(state == Socket::Done &&
                  i->data.GetDataSize() == 4)
                  {
                      cout << "packet received successfull" << endl;
                      //handle packet (omitted)
                  }
                  if(state == Socket::NotReady) continue;
          }
       }
}
}

int main()
{
ConnectionManager c;
c.run();
return 0;
}



Client:

Code: [Select]


   SocketTCP ssock;
   if(ssock.Connect(4223,"127.0.0.1",1.f))
   {
      Packet p;
      p << (Uint32)0x80A1DA12;
      ssock.Send(p);
      ssock.Close();
   }
   return 0;


Cause I rewrote the code to post it here there might be syntax flaws or so, but I think it will reproduce the error on my platform.

[Laurent]

Well, the idea is to get a code that I can copy / paste / execute / get the error directly, there's no point getting it if I have to spend time for making it work...

[zac]

So it would be nice to have some uncommented ~1200 lines for the whole server depending on some libs that will be kind of pain in the ass to compile on windows... not so sure...

This code is the part of the code without those dependencies and it is the part that fails...

I will fix the code so that you can copy/paste it...