[C++]Packet Loss

[sub1imina1]

My bot only receives like 3 packets at a time.  like ill type "/who <channel>" and it only gives me like 3 packets but theres like 40 people in the channel.  Or when i join a channel it only gives me like 3 showuser packets.  I dont understand why this is happening has anyone had this problem?

edit, code:

Code Select Expand


while(recvsize>0){
   m_MAINWIN=FindWindow(0, lpcWindow);

   memcpy(recvbuf, "\0", strlen(recvbuf));
   recvsize=recv(bnetfd, recvbuf, sizeof(recvbuf), 0);
   packetid=recvbuf[1];

   switch(packetid){
   case 0x31:
      switch(recvbuf[4]){
      case 1:updatescr("pass changed!\r\n");
      case 0:updatescr("pass failed!\r\n");
      }
      break;
   case 0x50:
      updatescr("responding to 0x50\r\n");
      strcpy(mpqname, recvbuf + 24);
      strcpy(hashcmd, recvbuf + 37);
      servertoken = *(unsigned long *)(recvbuf + 8);
      switch(Send_0x51(mpqname, hashcmd, servertoken, cdkey, bnetfd, packetbuf))
         {
         case 1: updatescr("Could not check versions\r\n"); break;
         case 2: updatescr("Could not decode CD-Key\r\n"); break;
         case 3: updatescr("Could not hash CD-Key\r\n"); break;
         }
      break;
   case 0x51:
      if(recvbuf[4] == 0x00){
         updatescr("Version and CD-Key verified\r\n");
         Send_0x3a(username, password, servertoken, bnetfd, packetbuf);
      break;
      }
   case 0x00:
      //packetbuf.sendpacket(bnetfd, 0x00);
      break;
   case 0x13:
      updatescr("FLOOOOOODDDDEEEDD!!!!!!!!!!!!!!!!!!!\n", xred);
      break;
   case 0x0f:
      parse(recvbuf, recvsize);
      break;
   case 0x3d:
      if(recvbuf[4] != 0x00){
         updatescr("AcCounT CreAte FaILed...\r\n", xred);
         break;
      }
      break;
   case 0x2a:
      if(recvbuf[4]==0x01){
         updatescr("aCcOUnT CReatEd!\r\n", xgreen);
         Send_0x3a(username, password, servertoken, bnetfd, packetbuf);
      }
      else{
         updatescr("AccOuNT CreaTE FailED...\r\n", xred);
         recvsize=-1;
      }
   case 0x3a:
      if(recvbuf[4] != 0x00){
      char createhash[1024]="";
      updatescr("Login failed, Attempting to create account...\n", xyell);
      HashData(password, strlen(password), createhash);
      packetbuf.insert(createhash, 5 * sizeof(DWORD));
      packetbuf.insert(username);
      packetbuf.sendpacket(bnetfd, 0x2a);
      break;
      }
      else{
      updatescr("Login accepted\r\n", xgreen);
      Send_0x14(bnetfd, packetbuf);
      updatescr("0x14 sent\r\n", xgreen);

      char p_0x0a[128]="";
      WORD p_0x0a_size=6+strlen(username);
      p_0x0a[0]=0xff;
      p_0x0a[1]=0x0a;
      memcpy(p_0x0a+2, &p_0x0a_size, 2);
      memcpy(p_0x0a+4, username, strlen(username));
      p_0x0a[5+strlen(username)]=0x00;
      send(bnetfd, p_0x0a, p_0x0a_size, 0);
      updatescr("0x0a sent\r\n", xgreen);

      char p_0x0c[128]="";
      WORD p_0x0c_size=9+strlen(homechannel);
      DWORD jflags=0x00;
      p_0x0c[0]=0xff;
      p_0x0c[1]=0x0c;
      *(unsigned short*)(p_0x0c+2)= (unsigned short)p_0x0c_size;
      memcpy(p_0x0c+4, &jflags, 4);
      memcpy(p_0x0c+8, homechannel, strlen(homechannel));
      p_0x0a[9+strlen(homechannel)]=0x00;
      send(bnetfd, p_0x0c, p_0x0c_size, 0);
      updatescr("0x0c sent\r\n", xgreen);
      abletoexecute=1;
      break;
      }
   }


[Skywing]

You are assuming that recv() will always return exactly one complete BNCS packet.  This is not the case.

You might want to read through the complete discussion here in order to gain a better understanding of the issue, and how to fix your code to avoid it.

[sub1imina1]

that was it.  thanks.

[sub1imina1]

one quick question... is it possible to recieve like half of a packet and have it continued in another?

[Kp]

one quick question... is it possible to recieve like half of a packet and have it continued in another?

Yes.  TCP is an octet-oriented stream.  It guarantees you'll get the bytes in the same order that the sender sent them, but it doesn't guarantee whether you'll get them quickly, or in the same groupings that the sender used.  You've already seen that it can stack up what was once distinct transmissions, and it's possible (though with battle.net, somewhat rare), for the reverse to happen.  Usually you'll see a partial message due to receiving so much that your receive buffer couldn't take the full item in one receive (i.e. bnet sends you 8kb, you use a 2kb buffer -- you're going to get fragmentation).  If your code is set up well, it'll be able to reintegrate the message properly and the higher level processors will never need to know or care that the message was received in two (or more) fragments.

[sub1imina1]

damn this is gonna be hard.  my approach is making a thread to constantly receive and '+=' it to a string.  then have another thread constantly searching for the byte 0xff in that string and get each packet out of the string.  is this a good or bad idea.

[Sargera]

damn this is gonna be hard.  my approach is making a thread to constantly receive and '+=' it to a string.  then have another thread constantly searching for the byte 0xff in that string and get each packet out of the string.  is this a good or bad idea.

Multi-threading is usually never a good idea, and for most of the things you'd do with it, there's alternative methods that are much better.

Here's a striped down version of RaiBot's code (you'll need DM's packet buffer):

Code Select Expand

PacketBuffer packetbuf2;

HANDLE events[2];
char tmp[32] = "";
char buffer[20000] = "";
char packetdata[5000] = "";
char packetid = 0;
unsigned short packetlen = 0;
unsigned long encryptvalue = 0;
unsigned long idletimer = 0, idletime = 0, lasttick = 0;
char mpqname[128] = "", hashcmd[256] = "", exeinfo[256] = "";
char uname[17] = "", name[32] = "", txt[256] = "", stats[512] = "", cdkeyname[512] = "", prog[8] = "", tmpproduct[5] = "";
unsigned long ping = 0, flags = 0, pingvalue = 0;

void ParseBnet(bool connected, char *username, char *password, char *cdkey, int verbyte, char *verbyte2, char *homechannel, char *gametype, SOCKET s) {

   char timestamp[11] = "";
   TimeStamp(timestamp);

   if(!connected) {
      printf("%s Error: Not connected to battle.net!\r\n", timestamp);
      return;
   }

   send(s, "\x1", 1, 0);
   packetbuf2.clear();
   packetbuf2.insert((int)0);
   packetbuf2.insert("68XI", 4);
   packetbuf2.insert(gametype, 4);
   packetbuf2.insert(verbyte);
   packetbuf2.insert((int)0);
   packetbuf2.insert((int)0);
   packetbuf2.insert((int)0);
   packetbuf2.insert((int)0);
   packetbuf2.insert((int)0);
   packetbuf2.insert("USA");
   packetbuf2.insert("United States");
   packetbuf2.sendpacket(s, 0x50);
   idletimer = GetTickCount() + 180000;

   printf("%s Sent packet 0x50... Product: %s, Verbyte: %s(%i)\r\n", timestamp, gametype, verbyte2, verbyte);

   while(connected) {
      idletime += GetTickCount() - lasttick;
      lasttick = GetTickCount();

      int buflen = 0;
      int recvlen = recv(s, buffer + buflen, sizeof(buffer) - buflen, 0);
      if(!recvlen || recvlen == SOCKET_ERROR) {
         printf("%s Error: SOCKET_ERROR! Possible ip ban!\r\n", timestamp);
         shutdownClient(s);
         return;
      }
      
      buflen += recvlen;

      while((int)buflen >= 4 && connected && (unsigned char)buffer[0] == 0xff) {
         packetid = buffer[1];
         packetlen = *(unsigned short *)(buffer + 2);
         memcpy(packetdata, buffer, packetlen);
         TimeStamp(timestamp);

         if(GetTickCount() >= idletimer && idletimer != 0) {
            char tmpbuf[256] = "";
            SYSTEMTIME a = FormatDWORD(idletime);
            printf("%s Idle Message Sent!\r\n", timestamp);
            idletimer = GetTickCount() + 180000;
         }

         switch(packetid) {

            case 0x00:
               // SID_NULL
               packetbuf2.sendpacket(s, 0x0);
               break;

            case 0x50:
               // SID_AUTH_INFO
               strcpy(mpqname, buffer + 24);
               strcpy(hashcmd, buffer + 37);
               encryptvalue = *(unsigned long *)(buffer + 8);
               switch(Send_0x51(mpqname, hashcmd, encryptvalue, cdkey, s)) {
                  case 0:
                     printf("%s Verifying Version and CD-Key...\r\n", timestamp);
                     break;

                  case 1:
                     printf("%s Could not check versions!\r\n", timestamp);
                     break;

                  case 2:
                     printf("%s Could not decode CD-Key!\r\n", timestamp);
                     break;

                  case 3:
                     printf("%s Could not hash CD-Key!\r\n", timestamp);
                     break;
               }
               break;

            case 0x51:
               // SID_AUTH_CHECK
               if(buffer[4] == 0x00) {
                  printf("%s Version and CD-Key verified...\r\n", timestamp);
                  Send_0x2d_0x14(s);
                  Send_0x3a(username, password, encryptvalue, s);
               }
               
               else {
                  printf("%s Version and CD-Key not verified!\r\n", timestamp);
                  
               }
               break;

            case 0x25:
               // SID_PING
               pingvalue = *(unsigned long *)(buffer + 4);
               packetbuf2.insert((int)pingvalue);
               packetbuf2.sendpacket(s, 0x25);
               break;

            case 0x3a:
               // SID_LOGONRESPONSE
               if(buffer[4] != 0x00){
                  printf("%s Login failed!\r\n", timestamp);
                  break;
               }
               printf("%s Login accepted!\r\n", timestamp);
               packetbuf2.insert(username);
               packetbuf2.insert(gametype);
               packetbuf2.sendpacket(s, 0x0a);
               packetbuf2.insert(gametype, 4);
               packetbuf2.sendpacket(s, 0x0b);
               packetbuf2.insert((int)2); // 0 = Non-existant, 1 = Product-specific, 2 = Home channel //
               packetbuf2.insert(homechannel);
               packetbuf2.sendpacket(s, 0x0c);
               break;

            case 0x0a:
               // SID_ENTERCHAT
               strcpy(uname, buffer + 4);
               printf("%s Logged on as %s\r\n", timestamp, uname);
               idletimer = GetTickCount() + 180000;
               lasttick = GetTickCount();
               idletime = 0;
               break;

            case 0x0f:
               switch(buffer[4]) {

                  case 0x01:
                     getvalues(buffer, &ping, &flags, name, txt);
                     strncpy(tmpproduct, txt, 4);
                     strcpy(stats, makelongclient(tmpproduct));
                     printf("%s In channel: %s:%i:%i, using %s!\r\n", timestamp, name, flags, ping, stats);
                     break;

                  /*case 0x02:
                     getvalues(buffer, &ping, &flags, name, txt);
                     strncpy(prog, txt, 4);
                     printf("%s joined with %dms and flags of %x; %s\r\n", name, ping, flags, stats);
                     break;
                  */

                  case 0x07:
                     getvalues(buffer, &ping, &flags, name, txt);
                     printf("%s Joined %s [flags: 0x%x]\r\n", timestamp, txt, flags);
                     break;


               } // switch(buffer[4]) {

         } // switch(packetid) {
         if((int)buflen - (int)packetlen < 0) buflen += packetlen;
         if((int)buflen - (int)packetlen >= 0)
         memmove(buffer, buffer + packetlen, buflen - packetlen);
         buflen -= packetlen;
         DoEvents();

      } // while((int)buflen >= 4 && connected && (unsigned char)buffer[0] == 0xff) {
      DoEvents();

   } // while(connteced) {

} // void ParseBnet() {

[Eibro]

Think about this for a second. You're not going to have a complete packet until recv returns. Why spawn an extra thread to check your buffer while recv is blocking? Do something like this;

Code Select Expand


struct PacketHeader {

    BYTE proto;
    BYTE id;
    WORD len;
};

WSABUF buffer;
buffer.buf = new char[1024];
buffer.len = 1024;

int recved = 0;

// ...

recved += recv( buffer );

while ( recved >= sizeof PacketHeader )  {
   PacketHeader* p = (PacketHeader*)buffer.buf;
   if ( recved >= p->len ) {
         // Full packet, handle it

         recved -= p->len;
   }
}

[CodeMaster]

Think about this for a second. You're not going to have a complete packet until recv returns. Why spawn an extra thread to check your buffer while recv is blocking?

Exactly, the packets have a length in the header for a reason. Use this to your advantage to construct the complete packet.

[sub1imina1]

the two threads thing worked but i just did it a different way and much simpler.

Code Select Expand


recvsize=recv(bnetfd, recvbuf, 4, 0);
plen=*(unsigned short*)(recvbuf+2);
strcpy(cPacket, recvbuf);
recv(bnetfd, (cPacket+4), plen-4, 0);
packetid=cPacket[1];


[Grok]

Multi-threading is usually never a good idea, and for most of the things you'd do with it, there's alternative methods that are much better.

Explain why multi-threading is usually never a good idea?

[dxoigmn]

the two threads thing worked but i just did it a different way and much simpler.

Code Select Expand


recvsize=recv(bnetfd, recvbuf, 4, 0);
plen=*(unsigned short*)(recvbuf+2);
strcpy(cPacket, recvbuf);
recv(bnetfd, (cPacket+4), plen-4, 0);
packetid=cPacket[1];


recv() is not guarenteed to recv the full length of your buffer.  It is possible that the first call to recv() could return 1 byte thusing causing plen to be arbitrary.  Also, it would be better to use memcpy() instead of strcpy() as I believe strcpy() using the null terminator as a reference where to stop and there can be quite a few of those in the packet thus truncating it.

[sub1imina1]

ok this should do the trick i hope?

Code Select Expand



   memcpy(cPacket, "\0", strlen(cPacket));
   ipos=recv(bnetfd, cPacket, 4, 0);
   if(ipos>=4){
      plen=*(unsigned short*)(recvbuf+2);
   }
   else{
      while(ipos<4){
         ipos+=recv(bnetfd, cPacket+ipos, 4-ipos, 0);
      }
   }
   while(ipos<plen){
      ipos+=recv(bnetfd, (cPacket+ipos), plen-ipos, 0);
   }
   packetid=cPacket[1];

   switch(packetid){...}
}


edit: made a little more efficient

[Banana fanna fo fanna]

Multi-threading is usually never a good idea, and for most of the things you'd do with it, there's alternative methods that are much better.

Explain why multi-threading is usually never a good idea?

Personally, I think multithreading makes things pretty confusing. I try to avoid it when I can, but it is neccessary in many situations.

[Skywing]

ok this should do the trick i hope?

Code Select Expand



   memcpy(cPacket, "\0", strlen(cPacket));
   ipos=recv(bnetfd, cPacket, 4, 0);
   if(ipos>=4){
      plen=*(unsigned short*)(recvbuf+2);
   }
   else{
      while(ipos<4){
         ipos+=recv(bnetfd, cPacket+ipos, 4-ipos, 0);
      }
   }
   while(ipos<plen){
      ipos+=recv(bnetfd, (cPacket+ipos), plen-ipos, 0);
   }
   packetid=cPacket[1];

   switch(packetid){...}
}


edit: made a little more efficient

recv can return zero or a negative value if the operation fails.  You should handle this case.
strlen requires that the string be null terminated; messages from Battle.net are not necessarily null terminated.
Make sure that you don't recv more data into cPacket than space is allocated for that variable.