2

I have created a short class for socket communication and want so send and receive multiple times to the same device (here as an example www.kernel.org in my case its a sensor connected over ethernet). The communication is working with HTTP and I tested the device with "PacketSender" and it works fine.

For the first try I send the request and get the expected response. Then I want to loop it, but then I can send as much and as diffrent requests as I want and I get nothing.

Here is my class (I left it only as a draft in the H-File and will make it more decent when it works)

#ifndef SOCKETCLASS_H_INCLUDED
#define SOCKETCLASS_H_INCLUDED

#include <windows.h>
#include <winsock2.h>

#include <stdio.h>

#pragma comment(lib, "Ws2_32.lib")

class WinSocket{
private:
    SOCKET s;
    long rc;
    int Status = 0;
    int PackageSize = 10;

public:

    WinSocket(){
       rc=startWinsock();
        if(rc!=0){
            printf("Error: startWinsock failed, ErCode %d \n",int(rc));
            Status= -1;
        }else{
            printf("Winsock ready!\n");
        }

    }

    int openSocket(char *IPv4, int Port){
        SOCKADDR_IN addr;

        int rc = 0;

        s = socket(AF_INET,SOCK_STREAM,0);
        if(s==INVALID_SOCKET){
            printf("Error: Socket could not be created! ErCode: %d\n",WSAGetLastError());
            return 1;
        }else{
            printf("Socket created\n");
        }

        memset(&addr,0,sizeof(SOCKADDR_IN));
        addr.sin_family=AF_INET;
        addr.sin_port=htons(80);    
        addr.sin_addr.s_addr=inet_addr(IPv4);    

        rc=connect(s,(SOCKADDR*)&addr,sizeof(SOCKADDR));
        if(rc==SOCKET_ERROR){
            printf("Error: Failed to connect, ErCode: %d\n",WSAGetLastError());
            return 1;
        }else{
            printf("Connected to %s...\n",IPv4);

        }

        return 0;
    }

    int sendPackage(char *Buffer, int BufferSize){
        int i,j;
        int result;
        int Pointer = 0;


        char *Package;
        Package = new char[PackageSize];

        for(i = 0; i < BufferSize; i+=PackageSize){
            for(j = 0; j < PackageSize; j++){
                Package[j] = Buffer[Pointer];
                Pointer++;
                if(BufferSize <= Pointer){
                    break;
                }
            }

            result = send(s,Package,PackageSize,0);
            if(result<0){
                printf("Error Occured!\n");
                return -1;
            }

        }
        delete[]Package;

        return 0;

    }



int readLine(char *Buffer, int *BufferSize){
    int MAX_BUFFERSIZE = 1024;
    int res = 0;
    char B[1];
    *BufferSize = 0;
    int Pointer = 0;

    for(int i=0;i<MAX_BUFFERSIZE;i++){
        res = recv(s,B,1,0);
        if(res > 0){
            if(B[0]!='\n'){
                //printf("%d;%s\n",res,B);
                Buffer[Pointer] = B[0];
                Pointer += 1;
            }else{
                Buffer[Pointer]='\n';
                *BufferSize = Pointer;
                return 0;
            }
        }else if(res == 0){
            return 1;
        }else{
            return -1;
        }
    }

    return 0;
}

int startWinsock(void){
    WSADATA wsa;
    return WSAStartup(MAKEWORD(2,0),&wsa);
}



      ~WinSocket(){
          closesocket(s);
      }
  };

  #endif // SOCKETCLASS_H_INCLUDED

As a main function i use the following code:

#include <stdio.h>
#include "SocketClass.h"

int main(){
    char buf[256];

    int i,j;

    int requestSize = 100;
    char request[100];

    char Buf[1024];
    int BufferSize = 1;

    WinSocket S;

    S.openSocket("147.75.44.153",80);
    for(int l = 0; l<10; l++){
        printf("Loop %d\n",l);
        printf("Sending: ");
        requestSize =  sprintf(request,"GET /faq.html HTTP/1.1\r\nHost: www.kernel.org\r\nConnection: close\r\n\r\n");
        printf("%d Bytes: \n",requestSize);
        S.sendPackage(request,100);
        for(j= 0; j < requestSize; j ++){
            printf("%c",request[j]);
        }
        printf("\n");

        Sleep(500);


        printf("Waiting for responset...\n");

        while(BufferSize!=0){
            printf("Received: ");
            S.readLine(Buf, &BufferSize);
            printf("%d Bytes: \n",BufferSize);

            for(int i=0;i<BufferSize;i++){
                    printf("%c",Buf[i]);
            }
            printf("\n");
        }
        if(BufferSize==0){
            printf("\nNothing more received...\n");
        }
        printf("Repeat!\n");
        Sleep(5000);
    }

    printf("Finished...\n");
    printf("Press ENTER to continue...\n");
    getchar();
    return 0;
}

I tested to open and close the socket every loop. This is sometimes working (so is there a non defined pointer I don't mentioned?), but I want a long duration of the connection.

What I do not understand is, why I can send multiple times but then receive only at the first sending any data.

I am using windows 7 and mingw (GNU GCC Compiler).

Thank you for your help.

T Rode
  • 21
  • 2

2 Answers2

1

In your HTTP request, you are sending a Connection: close header. That means the server WILL close its end of the TCP connection after sending the response. You are not detecting that condition so that you can close your current socket and re-connect to the server with a new socket on the next request.

You say you want the connection to remain open for multiple requests. That means you MUST utilize HTTP keep-alives (see RFC 2616 Section 8.1 and RFC 7230 Section 6.3). You need to remove the Connection: close header from your request (as HTTP 1.1 uses keep-alives by default), or else replace it with an explicit Connection: keep-alive header.

However, either way, you MUST read the response correctly (which you are not) in order to detect the correct end of the response (see RFC 2616 Section 4.4 and RFC 7230 Section 3.3.3) so you don't corrupt data between multiple responses on the same connection.

And you MUST pay attention to the Connection header in the server's response, as there is no guarantee that the server will actually honor a request for a keep-alive. If the server sends an HTTP 1.0 response without an explicit Connection: keep-alive header, or sends an HTTP 1.1 response with an explicit Connection: close header, or otherwise sends a message whose length is denoted by the server being required to close the connection, you MUST close your current socket and reconnect with a new socket on the next request.

Community
  • 1
  • 1
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • I do not understand, what you mean with correct end of the response? Does the connection fail because I have something in the queue? I added a close routine to the socket class and open the the socket and open and close the socket in the loop, but i got still no response in the second loop. – T Rode Jun 25 '18 at 12:30
  • Did you read the RFCs I linked to? They explain what I was referring to. If you want to send multiple requests over a single TCP connection, you need to read the responses correctly. You must read to the correct end of the response, no more, no less, or else you are going to corrupt the data you process. A `ReadLine()` loop can be used to read the HTTP response headers, but it is NOT the correct way to read the HTTP response body, not even close. – Remy Lebeau Jun 25 '18 at 15:46
  • yes. But when i read as long as the server sends something, i can not find a reason why the next request is getting no response (or sometimes only the header and no message body) there is no acknowledgement after reading or sending, or am i wrong? – T Rode Jun 28 '18 at 08:41
  • @TRode you MUST MUST follow the protocol! I cannot stress that enough. You cannot just *blindly* read until there is nothing left to read. You must read the response headers first and analyze them to know if a response body is present and in what format it is encoded in so you know HOW to read it and WHEN to stop reading it. See [this pseudo code](https://stackoverflow.com/a/7234357/65863) for the correct reading logic. – Remy Lebeau Jun 28 '18 at 15:25
  • Ok, first thank you for the sake of helping... I will explain my issue more clearly. I connect to a sensor using HTTP and i get NO Content-Length. When I can not receive a header how can i analize the content-length? I send the first recv() - Command. Than i get the Header "HTTP/1.1 400 Bad Reques Server: nginx Date: xxx Content-Type: text/html Content-Length 166 Connection: keep-alive \empty line ... <\html> so. when i read after the header file is complete my 166 bytes i can still not recv any aditional content by sendig a request by sending a GET-command. – T Rode Jul 09 '18 at 08:19
  • The request for the Sensor: GET /mOn.xml HTTP/1.1 The Header i receive from the Sensor: HTTP/1.1 200 OK Conection: keep-alive Access-Control-Allow-Origin: * Content-Type: text/xml Cache-Control: no-cache /empty line – T Rode Jul 09 '18 at 08:27
  • @TRode that `400` reply has a `Content-Length: 166` header, so the end of the reply is exactly 166 bytes after the headers, so that is how many bytes you must read and then stop. The reply also has a `Connection: keep-alive` header, so you can send a new request on the same socket connection. The next `200` reply has no `Content-Length` or `Transfer-Encoding` header, so per the rules of HTTP, the end of the reply is denoted by the connection being closed by the server, regardless of `Connection: keep-alive` being present. – Remy Lebeau Jul 09 '18 at 16:09
  • @TRode if you are following the rules and still having trouble, then you likely still have faulty logic in your code. Please edit your question to show your latest code. – Remy Lebeau Jul 09 '18 at 16:17
0

In the 2nd loop send() returns an error for me: WSAECONNABORTED.

Sockets are a real fun when it comes to errors, so you should check the real errors behind a failure of send() etc with the real error numbers. Exchange the lines

    if(result<0){
        printf("Error Occured!\n");
        return -1;
    }

with

    if(result < 0) {
        printf( "Error Occured: %i\n", WSAGetLastError() );
        return -1;
    }

to get the real error.

WSAECONNABORTED means that probably there's a timeout or something wrong with the protocol. Windows socket errors

I think this is caused by the server automatically disconnecting after fulfilling the request, so that there needs to be a new connection for each request. There is no persistent connection.

I looked in my own code and I did it in that order:

  • connect
  • send request
  • receive data
  • wait for server to disconnect

Besides of that, there seems to be some errors in your code. E.g. in result = send(s,Package,PackageSize,0); you always send 10 chars, even if the loop before was left because of if(BufferSize <= Pointer) - this means you send some rubbish chars to the server. This can be avoided by

for(i = 0; i < BufferSize; i += PackageSize) {
    int packageSize = 0;
    for(j = 0; j < PackageSize; j++) {
        Package[j] = Buffer[Pointer];
        packageSize = Pointer;
        Pointer++;
        if(BufferSize <= Pointer) {
            break;
        }
    }
    result = send( s, Package, packageSize, 0 );

    if(result < 0) {
        printf( "Error Occured: %i\n", WSAGetLastError() );
        return -1;
    }

}
user2328447
  • 1,807
  • 1
  • 21
  • 27
  • 1
    Thank you. When I add "connection: keep-alive" or close and reopen the socket in each loop i got rid of WSAECONNABORTED. And i also added in every error-if-statement the port with WSAGetLastError(). – T Rode Jun 25 '18 at 12:32