Sending binary file via Winsock doesn't copy first 4 characters

Question

So, of course, the file is not complete, but the actual size (not size on disk) is exactly the same.

Here is the complete code of the server and client. In this example, I am copying the file from C:\temp\IMG_8526.jpg to E:\temp\newjpg.jpg.

SERVER:

#include <winsock2.h>
#include <windows.h>
#include <ws2tcpip.h>
#include <strsafe.h>
#include <process.h>
#include <mswsock.h>

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

SOCKET SetUpListener(CONST CHAR *pcAddress, CONST INT nPort)
{
    INT iBind = 0; ULONG nInterfaceAddr = 0;
    inet_pton(AF_INET, pcAddress, &nInterfaceAddr);
    if (nInterfaceAddr != INADDR_NONE) 
        {
        SOCKET sd = socket(AF_INET, SOCK_STREAM, 0);
        if (sd != INVALID_SOCKET) 
            {
            sockaddr_in sinInterface;
            sinInterface.sin_family = AF_INET;
            sinInterface.sin_addr.s_addr = nInterfaceAddr;
            sinInterface.sin_port = nPort;            
            iBind = bind(sd, (sockaddr*) &sinInterface, sizeof(sockaddr_in));
            if (iBind != SOCKET_ERROR)
                {
                listen(sd, SOMAXCONN);
                return sd;
                }
            }
        }
    return INVALID_SOCKET;
}

int readBytes(CONST SOCKET s, void *buffer, int buflen)
{
    INT total = 0; UCHAR *pbuf = (UCHAR *) buffer;
    while (buflen > 0)
        {
        INT iResult = recv(s, (CHAR *) pbuf, buflen, 0);
        if (iResult < 0)
            {
            if (WSAGetLastError() == WSAEWOULDBLOCK)
                continue;
            return SOCKET_ERROR;
            }
        else if (iResult == 0)
            return 0;
        else
            {
            pbuf += iResult;
            buflen -= iResult;
            total += iResult;
            }
        }
    return total;
}

BOOL ReceiveDoWorkThenReturnDataToClient(CONST SOCKET sd)
{    
    CHAR acReadBuffer[1024]; 
    WCHAR sTempFileOut[MAX_PATH] = L"E:\\temp\\newjpg.jpg";
    INT nReadBytes;
    do 
        {
        nReadBytes = recv(sd, acReadBuffer, 1024, 0);
        if (nReadBytes > 0) 
            {
            INT nSentBytes = 0;
            while (nSentBytes < nReadBytes)
                {
                DeleteFile(sTempFileOut);
                FILE* fp = NULL;
                errno_t err = _wfopen_s(&fp, sTempFileOut, L"wb");
                if (fp == NULL)
                    return 1;
                ULONG FileSize;
                int iResult = readBytes(sd, &FileSize, sizeof(FileSize));
                if (iResult <= 0)
                    {
                    fclose(fp);
                    return 1;
                    }
                FileSize = ntohl(FileSize);
                CHAR mfcc[1024];
                while (FileSize > 0)
                    {
                    int Received = readBytes(sd, mfcc, min(sizeof(mfcc), FileSize));
                    if (Received <= 0)
                        {
                        fclose(fp);
                        return 1;
                        }
                    if (fwrite(mfcc, 1, Received, fp) != Received)
                        {
                        fclose(fp);
                        return 1;
                        }
                    FileSize -= Received;
                    }

                fflush(fp);
                fclose(fp);
 
                INT nTemp = send(sd, "1" + nSentBytes, (INT)strlen("1") - nSentBytes, 0);
                if (nTemp > 0)
                    nSentBytes += nTemp;
                else if (nTemp == SOCKET_ERROR)
                    break;
                else
                    break;
                }
            }
        else if (nReadBytes == SOCKET_ERROR) 
            return FALSE;
        }
    while (nReadBytes != 0);

    return TRUE;
}

static BOOL ShutdownConnection(SOCKET sd)
{
    CONST DWORD BUFFERSIZE = 4096;
    if (shutdown(sd, SD_SEND) == SOCKET_ERROR) 
        return FALSE;
    CHAR acReadBuffer[BUFFERSIZE];
    while (1) 
        {
        INT nNewBytes = recv(sd, acReadBuffer, BUFFERSIZE, 0);
        if (nNewBytes == SOCKET_ERROR) 
            return FALSE;
        else if (nNewBytes == 0) 
            break;
        }
    if (closesocket(sd) == SOCKET_ERROR) 
        return FALSE;
    return TRUE;
}

static DWORD WINAPI ThreadHandler(void* sd_)
{
    INT nRetval = 0;
    SOCKET sd = (SOCKET) sd_;
    if (!ReceiveDoWorkThenReturnDataToClient(sd)) 
        nRetval = 3;
    if (!ShutdownConnection(sd)) 
        nRetval = 4;
    return nRetval;
}

static unsigned WINAPI ListenAndAccept(void *)
{
    sockaddr_in sinRemote;
    INT nAddrSize = sizeof(sinRemote);
    DWORD nThreadID;
    SOCKET ListeningSocket = SetUpListener("localhost", htons(43210));
    if (ListeningSocket == INVALID_SOCKET)
        return 3;
    while (1)
        {
        SOCKET sd = accept(ListeningSocket, (sockaddr*)&sinRemote, &nAddrSize);
        if (sd != INVALID_SOCKET)
            CreateThread(0, 0, &ThreadHandler, (void*)sd, 0, &nThreadID);
        else
            break;
        }
    return 0;
}

INT main()
{
    WSADATA wsaData = { 0 };
    if (WSAStartup(MAKEWORD(1, 1), &wsaData) != ERROR_SUCCESS) return 0;
    UINT threadID = 0;
    HANDLE hThread = (HANDLE)_beginthreadex(NULL, 0, &ListenAndAccept, NULL, 0, &threadID);
    WaitForSingleObject(hThread, INFINITE);
    CloseHandle(hThread);
    while (1)
        ;
}

CLIENT (sends the jpg file to the server)

#include <winsock2.h>
#include <windows.h>
#include <mswsock.h>
#include <ws2tcpip.h>
#include <strsafe.h>
#include <windns.h>
#include <process.h> 
#include <windns.h>

#pragma comment(lib, "Ws2_32.lib")      // More than one instance overloaded function has C linkage
#pragma comment(lib, "dnsapi.lib")  // needed for DnsFree API


int sendBytes(SOCKET s, void* buffer, int buflen)
{
    INT total = 0;

    unsigned char* pbuf = (unsigned char*)buffer;
    while (buflen > 0)
        {
        int iResult = send(s, (char*)pbuf, buflen, 0);
        if (iResult < 0)
            {
            if (WSAGetLastError() == WSAEWOULDBLOCK)
                continue;
            return SOCKET_ERROR;
            }
        else if (iResult == 0)
            return 0;
        else
            {
            pbuf += iResult;
            buflen -= iResult;
            total += iResult;
            }
        }

    return total;
}

DWORD File_Transfer_Manip(SOCKET ConnectSocket, WCHAR *sFileToSend)
{
    CHAR Buffer[1024];
    DWORD dwWSA = 0;

    FILE* fp = NULL;
    errno_t err = _wfopen_s(&fp, sFileToSend, L"rb");
    if (!fp)
        return err;

    fseek(fp, 0, SEEK_END);
    ULONG FileSize = ftell(fp);
    fseek(fp, 0, SEEK_SET);

    ULONG tmpFileSize = htonl(FileSize);
    int iResult = sendBytes(ConnectSocket, &tmpFileSize, sizeof(tmpFileSize));
    if (iResult <= 0)
        {
        fclose(fp);
        return 1;
        }

    while (FileSize > 0)
        {
        LONG Size = fread(Buffer, 1, min(sizeof(Buffer), FileSize), fp);
        if (Size <= 0)
            {
            fclose(fp);
            return 2;
            }

        if (sendBytes(ConnectSocket, Buffer, Size) != Size)
            {
            fclose(fp);
            return 3;
            }

        FileSize -= Size;
        }

    fclose(fp);
    return 0;
}

DWORD SendWinsockCommandToRemoteServer(CONST DWORD dwCharsAllocatedReturnString, WCHAR** sStringReturned)
{
    SOCKET ConnectSocket = INVALID_SOCKET;
    INT recvbuflen = 4096;
    DWORD dwIPv4 = 0, dwErr = 0;

    SecureZeroMemory(*sStringReturned, dwCharsAllocatedReturnString * sizeof(CHAR));

    ConnectSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    if (ConnectSocket == INVALID_SOCKET)
        return WSAGetLastError();

    inet_pton(AF_INET, "127.0.0.1", &dwIPv4);

    sockaddr_in clientService{};
    clientService.sin_family = AF_INET;
    clientService.sin_addr.s_addr = dwIPv4;
    clientService.sin_port = htons(43210);

    // If no error occurs, connect returns zero.
    INT iResult = connect(ConnectSocket, (SOCKADDR*)&clientService, sizeof(clientService));
    if ((iResult != 0) || (iResult == SOCKET_ERROR))
        {
        closesocket(ConnectSocket);
        return 1;
        }

    DWORD dwResult = File_Transfer_Manip(ConnectSocket, L"c:\\temp\\IMG_8526.jpg");

    iResult = shutdown(ConnectSocket, SD_SEND);
    if (iResult == SOCKET_ERROR)
        {
        closesocket(ConnectSocket);
        return 6;
        }

    closesocket(ConnectSocket);
    return 0;
}

int main()
{
    DWORD dwDataAllocatedSize = 1024;
    DWORD dwErr = 0;
    WCHAR* sData = new WCHAR[dwDataAllocatedSize]();
    WCHAR* sRV = new WCHAR[dwDataAllocatedSize]();
    WSADATA wsaData{};
    INT iResult = WSAStartup(MAKEWORD(2, 2), &wsaData);

    SendWinsockCommandToRemoteServer(dwDataAllocatedSize, &sRV);

    delete[] sRV;
    delete[] sData;

    wprintf(L"Press any key to end."); getchar();

    return 0;
}

Both are fully running examples. Any advice is appreciated on WHY the first part of the file isn't being copied? (looks like 4 characters when I bring it into Notepad++).

If I do copy the first four characters from the IMG_8526.jpg file into the newjpg.jpg file using Notepad++, then the image works.

in ReceiveDoWorkThenReturnDataToClient, the value for iResult the first time through is 4, so maybe that has to do with it?

This code was adapted from @RemyLebeau's reply at Winsock upload file to remote server

You'll be glad to hear you don't need anyone's help to figure this out, just a tool you already have: your debugger! This is exactly what a debugger is for. It [runs your program, one line at a time, and shows you what's happening](https://stackoverflow.com/questions/25385173/), this is something that's every C++ developer must know how to do. With your debugger's help you'll able to quickly find all problems in this and all future programs you write, without having to ask anyone for help. Have you tried using your debugger, already? If not, why not? What did your debugger show you? — Sam Varshavchik, Jul 09 '22 at 16:57
Debugger showed in `ReceiveDoWorkThenReturnDataToClient`, the value for `iResult` the first time through is 4. I appreciate the sarcasm :) — JeffR, Jul 09 '22 at 16:59
Since there are two programs here, both of them need debugging concurrently, and information from both debugging sessions correlated. It should be possible to follow the sender, step by step, through its task of sending the file, one step at a time, concurrently with doing the same on the receiving side, also step by step, and see where things get derailed. It's one thing if it's a short program, one or two pages in size. But how likely is that someone here would copy this whole thing, compile, run, and debug it themselves? Not likely. Why would they do that? It's up to you, I'm afraid! — Sam Varshavchik, Jul 09 '22 at 17:05
Many people help on here with full programs. even if you think they wouldn't. Have a nice day! — JeffR, Jul 09 '22 at 17:12
@sam Yes, sure, that sounds tedious. And it is, if you believe that GDB were a debugger. Luckily, on Windows, we actually have debuggers. Visual Studio's debugger, for example, can attach to multiple processes at once. — IInspectable, Jul 09 '22 at 17:13
If gdb isn't a debugger, I must've imagined using it as one, for the last 20+ years, or so. — Sam Varshavchik, Jul 09 '22 at 17:14

Remy Lebeau · Accepted Answer · 2022-07-09T18:37:08.807

On the server side, ReceiveDoWorkThenReturnDataToClient() is running a loop where each iteration is first reading an arbitrary buffer of bytes from the socket and discarding that buffer before then trying to receive a file.

When the client connects, it immediately sends the file's size followed by the file's data. But the server will throw away the first handful of bytes of that transmission, thus corrupting the file size and the beginning of the file.

You need to get rid of that initial arbitrary read on each loop iteration. In fact, you should just get rid of the loop altogether, since the client is sending only 1 file and then disconnecting.

It also looks like you are running a 2nd loop that tries to acknowledge each buffer received from the client. That is completely unnecessary in TCP, since it already has its own acknowledgement system built-in at the transmission layer. Not to mention, you are not even sending your acknowledgement correctly anyway (ie, "1" + nSentBytes doesn't do what you think it does). You don't need to acknowledge each buffer, but you could certainly send back a final reply at the end to let the client know whether or not the file was received and saved correctly.

Try something more like this instead:

BOOL ReceiveDoWorkThenReturnDataToClient(CONST SOCKET sd)
{    
    BOOL ReceiveSuccessful = FALSE;

    WCHAR sTempFileOut[MAX_PATH] = L"E:\\temp\\newjpg.jpg";
    DeleteFile(sTempFileOut);

    FILE* fp = NULL;
    errno_t err = _wfopen_s(&fp, sTempFileOut, L"wb");
    if (fp == NULL)
    {
        send(sd, (char*)&ReceiveSuccessful, 1, 0);
        return 1;
    }

    ULONG FileSize;
    int iResult = readBytes(sd, &FileSize, sizeof(FileSize));
    if (iResult <= 0)
    {
        fclose(fp);
        send(sd, (char*)&ReceiveSuccessful, 1, 0);
        return 1;
    }

    FileSize = ntohl(FileSize);
    CHAR mfcc[1024];
    while (FileSize > 0)
    {
        int Received = readBytes(sd, mfcc, min(sizeof(mfcc), FileSize));
        if (Received <= 0)
        {
            fclose(fp);
            send(sd, (char*)&ReceiveSuccessful, 1, 0);
            return 1;
        }
        if (fwrite(mfcc, 1, Received, fp) != Received)
        {
            fclose(fp);
            send(sd, (char*)&ReceiveSuccessful, 1, 0);
            return 1;
        }
        FileSize -= Received;
    }

    fflush(fp);
    fclose(fp);
 
    ReceiveSuccessful = TRUE;
    send(sd, (char*)&ReceiveSuccessful, 1, 0);

    return TRUE;
}

Sending binary file via Winsock doesn't copy first 4 characters

1 Answers1