C++file transfer with TCP protocol











up vote
-1
down vote

favorite












I'm currently writing a server and client app that attemps to transfer a screenshot but it's not working properly. I implemented it like this.



SOCKET sock;
char buf[4096];

DWORD WINAPI thread_function()
{
bool file_transfer = false;
bool loop = true;
while (1)
{
ZeroMemory(buf, 4096);
int bytesReceived = recv(sock, buf, 4096, 0);
if (bytesReceived > 0)
{
std::string received(buf, 0, bytesReceived);
if (received == "Sending file.")
{
file_transfer = true;
}

if (file_transfer == false)
{
std::cout << "nSERVER> " << std::string(buf, 0, bytesReceived) << std::endl;
std::cout << "> ";
}
else if (file_transfer == true)
{
loop = true;
TCHAR *szfname = "screenshot.bmp";
FILE* f = fopen(szfname, "wb");
if (NULL == f)
{
std::cerr << "Error opening file" << std::endl;
return 1;
}
while ((bytesReceived = recv(sock, buf, 4096, 0)) > 0 && loop == true)
{
received = buf;
if (received == "File transfer completed !")
{
loop = false;
std::cout << "File transfer completed !" << std::endl;
std::cout << "> ";
}
else
{
fwrite(buf, 1, bytesReceived, f);
}
}
file_transfer = false;
}
}
}
}


I call the function with this



CreateThread(0, 0, (LPTHREAD_START_ROUTINE)thread_function, 0, 0, 0);


The thing is I believe this is not a very clean way of doing it and also it's not working perfectly. After a file is received I don't correctly receive what the server is sending.



This is the server code wich I think is fine.



            send(clientSocket, TEXT("Attempting to take a screenshot."), sizeof(TEXT("Attempting to take a screenshot...")), 0);
HWND win = GetDesktopWindow();
HDC dc = GetDC(win);
if (HDCToFile("screenshot.bmp", dc, { 0, 0, 1920, 1080 }) == true)
{
send(clientSocket, TEXT("Sending file."), sizeof(TEXT("Sending file.")), 0);
FILE *fp = fopen("screenshot.bmp", "rb");
if (fp == NULL)
{
std::cerr << "Error : Cannot open file." << std::endl;
return 1;
}
while (1)
{
char buff[4096] = { 0 };
int nread = fread(buff, 1, 4096, fp);
if (nread > 0)
{
send(clientSocket, buff, sizeof(buff), 0);
}
if (nread < 4096)
{
if (feof(fp))
{
std::cout << "File transfer completed !" << std::endl;
send(clientSocket, TEXT("File transfer completed !"), sizeof(TEXT("File transfer completed !")), 0);
}
if (ferror(fp))
std::cerr << "Error reading." << std::endl;
break;
}
}
}
else
{
send(clientSocket, TEXT("Screen capture failed...."), sizeof(TEXT("Screen capture failed....")), 0);
}


Thanks for your time and help.










share|improve this question




















  • 1




    1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
    – UKMonkey
    Nov 12 at 16:24








  • 1




    Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
    – Useless
    Nov 12 at 16:46












  • This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
    – programme-zero
    Nov 12 at 16:56















up vote
-1
down vote

favorite












I'm currently writing a server and client app that attemps to transfer a screenshot but it's not working properly. I implemented it like this.



SOCKET sock;
char buf[4096];

DWORD WINAPI thread_function()
{
bool file_transfer = false;
bool loop = true;
while (1)
{
ZeroMemory(buf, 4096);
int bytesReceived = recv(sock, buf, 4096, 0);
if (bytesReceived > 0)
{
std::string received(buf, 0, bytesReceived);
if (received == "Sending file.")
{
file_transfer = true;
}

if (file_transfer == false)
{
std::cout << "nSERVER> " << std::string(buf, 0, bytesReceived) << std::endl;
std::cout << "> ";
}
else if (file_transfer == true)
{
loop = true;
TCHAR *szfname = "screenshot.bmp";
FILE* f = fopen(szfname, "wb");
if (NULL == f)
{
std::cerr << "Error opening file" << std::endl;
return 1;
}
while ((bytesReceived = recv(sock, buf, 4096, 0)) > 0 && loop == true)
{
received = buf;
if (received == "File transfer completed !")
{
loop = false;
std::cout << "File transfer completed !" << std::endl;
std::cout << "> ";
}
else
{
fwrite(buf, 1, bytesReceived, f);
}
}
file_transfer = false;
}
}
}
}


I call the function with this



CreateThread(0, 0, (LPTHREAD_START_ROUTINE)thread_function, 0, 0, 0);


The thing is I believe this is not a very clean way of doing it and also it's not working perfectly. After a file is received I don't correctly receive what the server is sending.



This is the server code wich I think is fine.



            send(clientSocket, TEXT("Attempting to take a screenshot."), sizeof(TEXT("Attempting to take a screenshot...")), 0);
HWND win = GetDesktopWindow();
HDC dc = GetDC(win);
if (HDCToFile("screenshot.bmp", dc, { 0, 0, 1920, 1080 }) == true)
{
send(clientSocket, TEXT("Sending file."), sizeof(TEXT("Sending file.")), 0);
FILE *fp = fopen("screenshot.bmp", "rb");
if (fp == NULL)
{
std::cerr << "Error : Cannot open file." << std::endl;
return 1;
}
while (1)
{
char buff[4096] = { 0 };
int nread = fread(buff, 1, 4096, fp);
if (nread > 0)
{
send(clientSocket, buff, sizeof(buff), 0);
}
if (nread < 4096)
{
if (feof(fp))
{
std::cout << "File transfer completed !" << std::endl;
send(clientSocket, TEXT("File transfer completed !"), sizeof(TEXT("File transfer completed !")), 0);
}
if (ferror(fp))
std::cerr << "Error reading." << std::endl;
break;
}
}
}
else
{
send(clientSocket, TEXT("Screen capture failed...."), sizeof(TEXT("Screen capture failed....")), 0);
}


Thanks for your time and help.










share|improve this question




















  • 1




    1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
    – UKMonkey
    Nov 12 at 16:24








  • 1




    Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
    – Useless
    Nov 12 at 16:46












  • This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
    – programme-zero
    Nov 12 at 16:56













up vote
-1
down vote

favorite









up vote
-1
down vote

favorite











I'm currently writing a server and client app that attemps to transfer a screenshot but it's not working properly. I implemented it like this.



SOCKET sock;
char buf[4096];

DWORD WINAPI thread_function()
{
bool file_transfer = false;
bool loop = true;
while (1)
{
ZeroMemory(buf, 4096);
int bytesReceived = recv(sock, buf, 4096, 0);
if (bytesReceived > 0)
{
std::string received(buf, 0, bytesReceived);
if (received == "Sending file.")
{
file_transfer = true;
}

if (file_transfer == false)
{
std::cout << "nSERVER> " << std::string(buf, 0, bytesReceived) << std::endl;
std::cout << "> ";
}
else if (file_transfer == true)
{
loop = true;
TCHAR *szfname = "screenshot.bmp";
FILE* f = fopen(szfname, "wb");
if (NULL == f)
{
std::cerr << "Error opening file" << std::endl;
return 1;
}
while ((bytesReceived = recv(sock, buf, 4096, 0)) > 0 && loop == true)
{
received = buf;
if (received == "File transfer completed !")
{
loop = false;
std::cout << "File transfer completed !" << std::endl;
std::cout << "> ";
}
else
{
fwrite(buf, 1, bytesReceived, f);
}
}
file_transfer = false;
}
}
}
}


I call the function with this



CreateThread(0, 0, (LPTHREAD_START_ROUTINE)thread_function, 0, 0, 0);


The thing is I believe this is not a very clean way of doing it and also it's not working perfectly. After a file is received I don't correctly receive what the server is sending.



This is the server code wich I think is fine.



            send(clientSocket, TEXT("Attempting to take a screenshot."), sizeof(TEXT("Attempting to take a screenshot...")), 0);
HWND win = GetDesktopWindow();
HDC dc = GetDC(win);
if (HDCToFile("screenshot.bmp", dc, { 0, 0, 1920, 1080 }) == true)
{
send(clientSocket, TEXT("Sending file."), sizeof(TEXT("Sending file.")), 0);
FILE *fp = fopen("screenshot.bmp", "rb");
if (fp == NULL)
{
std::cerr << "Error : Cannot open file." << std::endl;
return 1;
}
while (1)
{
char buff[4096] = { 0 };
int nread = fread(buff, 1, 4096, fp);
if (nread > 0)
{
send(clientSocket, buff, sizeof(buff), 0);
}
if (nread < 4096)
{
if (feof(fp))
{
std::cout << "File transfer completed !" << std::endl;
send(clientSocket, TEXT("File transfer completed !"), sizeof(TEXT("File transfer completed !")), 0);
}
if (ferror(fp))
std::cerr << "Error reading." << std::endl;
break;
}
}
}
else
{
send(clientSocket, TEXT("Screen capture failed...."), sizeof(TEXT("Screen capture failed....")), 0);
}


Thanks for your time and help.










share|improve this question















I'm currently writing a server and client app that attemps to transfer a screenshot but it's not working properly. I implemented it like this.



SOCKET sock;
char buf[4096];

DWORD WINAPI thread_function()
{
bool file_transfer = false;
bool loop = true;
while (1)
{
ZeroMemory(buf, 4096);
int bytesReceived = recv(sock, buf, 4096, 0);
if (bytesReceived > 0)
{
std::string received(buf, 0, bytesReceived);
if (received == "Sending file.")
{
file_transfer = true;
}

if (file_transfer == false)
{
std::cout << "nSERVER> " << std::string(buf, 0, bytesReceived) << std::endl;
std::cout << "> ";
}
else if (file_transfer == true)
{
loop = true;
TCHAR *szfname = "screenshot.bmp";
FILE* f = fopen(szfname, "wb");
if (NULL == f)
{
std::cerr << "Error opening file" << std::endl;
return 1;
}
while ((bytesReceived = recv(sock, buf, 4096, 0)) > 0 && loop == true)
{
received = buf;
if (received == "File transfer completed !")
{
loop = false;
std::cout << "File transfer completed !" << std::endl;
std::cout << "> ";
}
else
{
fwrite(buf, 1, bytesReceived, f);
}
}
file_transfer = false;
}
}
}
}


I call the function with this



CreateThread(0, 0, (LPTHREAD_START_ROUTINE)thread_function, 0, 0, 0);


The thing is I believe this is not a very clean way of doing it and also it's not working perfectly. After a file is received I don't correctly receive what the server is sending.



This is the server code wich I think is fine.



            send(clientSocket, TEXT("Attempting to take a screenshot."), sizeof(TEXT("Attempting to take a screenshot...")), 0);
HWND win = GetDesktopWindow();
HDC dc = GetDC(win);
if (HDCToFile("screenshot.bmp", dc, { 0, 0, 1920, 1080 }) == true)
{
send(clientSocket, TEXT("Sending file."), sizeof(TEXT("Sending file.")), 0);
FILE *fp = fopen("screenshot.bmp", "rb");
if (fp == NULL)
{
std::cerr << "Error : Cannot open file." << std::endl;
return 1;
}
while (1)
{
char buff[4096] = { 0 };
int nread = fread(buff, 1, 4096, fp);
if (nread > 0)
{
send(clientSocket, buff, sizeof(buff), 0);
}
if (nread < 4096)
{
if (feof(fp))
{
std::cout << "File transfer completed !" << std::endl;
send(clientSocket, TEXT("File transfer completed !"), sizeof(TEXT("File transfer completed !")), 0);
}
if (ferror(fp))
std::cerr << "Error reading." << std::endl;
break;
}
}
}
else
{
send(clientSocket, TEXT("Screen capture failed...."), sizeof(TEXT("Screen capture failed....")), 0);
}


Thanks for your time and help.







c++ sockets client-server file-transfer






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Nov 12 at 18:00

























asked Nov 12 at 16:13









programme-zero

13




13








  • 1




    1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
    – UKMonkey
    Nov 12 at 16:24








  • 1




    Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
    – Useless
    Nov 12 at 16:46












  • This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
    – programme-zero
    Nov 12 at 16:56














  • 1




    1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
    – UKMonkey
    Nov 12 at 16:24








  • 1




    Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
    – Useless
    Nov 12 at 16:46












  • This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
    – programme-zero
    Nov 12 at 16:56








1




1




1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
– UKMonkey
Nov 12 at 16:24






1. threads add problems of their own, so start with removing that. 2. read this 3. once you have things working without the thread, you can consider adding it.
– UKMonkey
Nov 12 at 16:24






1




1




Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
– Useless
Nov 12 at 16:46






Your receive-side check if (received == "Sending file.") is wrong. Assuming TCP, the string received can be a full 4Kb buffer, because boundaries between send calls are not preserved when calling recv. String comparison doesn't stop at the nul terminator if there is one. You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload).
– Useless
Nov 12 at 16:46














This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
– programme-zero
Nov 12 at 16:56




This is indeed TCP protocol. "You should consider separating your transport layer (big lumps of data received from the TCP stream) from your parsing layer (distinguishing status or control messages from the payload)." I understand what you mean but I honestly have no idea of how to implement that. For the person who linked me "How to debug small programs" if I had an idea of why this is not working I wouldn't be asking. I don't know a lot about sockets since I just started using them. My question isn't very specific because I don't know where the problem is at all.
– programme-zero
Nov 12 at 16:56












1 Answer
1






active

oldest

votes

















up vote
1
down vote













TCP is a streaming protocol. It has no concept of messages, so when the server sends "Sending file." there is no separation between the string and the beginning of the file being sent. Everything just goes into the stream one byte after the next and when the network stack decides it's time, usually because a packet has been filled or it's been too long since data was last added, a packet is sent, possibly containing multiple messages.



So



int bytesReceived = recv(sock, buf, 4096, 0);


very likely reads the full 4096 bytes, Attempting to take a screenshot.Sending file. plus the first four thousand-or-so bytes of the bitmap. The client code consumes the string and discards the rest of the buffer.



You need to establish a communication protocol that sits between the the socket and the writing of the file. There are a whole bunch of different ways to handle this. Common tricks for reading strings are




  1. Write the length of the string before writing the string so that the protocol handler knows how many bytes to read ahead of the time


Sender



uint16_t len = str.length(); // size is exactly 16 bits
len = htons(len); // endian is known
int sent = send(sock, (char*)&len, sizeof(len), 0);
// test sent for success (did not fail, sent all the bytes)
sent = send(sock, str.c_str(), len, 0);
// test sent for success (did not fail, sent all the bytes)
// may need to loop here if the string is super long.


Receiver



uint16_t len;
int recd = recv(sock, (char*)&len, sizeof(len), MSG_WAITALL);
// test recd for success (did not fail, read all the bytes)
// MSG_WAITALL will read exactly the right number of bytes or die trying.
len = ntohs(len); // ensure correct endian
std::string msg(len, ' '); // allocate a big enough string
char * msgp = &msg[0]; // or msg.data() if C++17 or better.
// Never seen &msg[0] fail, but this is not guaranteed by C++
while (len) // sometimes you want an extra exit condition here to bail out early
{
recd = recv(sock, msgp, len, 0);
// test recd for success
len -= recd;
msgp += recd;
}



  1. Insert a canary value so that the protocol handler knows when to stop reading. The null terminator works well here. The protocol reads up until it finds the null and preserves the remainder of what's read for later consumption. No code example here because this can be done many, many different ways.

  2. Not using strings and sending integer code messages instead. Eg:




enum messageID
{
TAKING_SCREENSHOT,
SENDING_FILE,
EATING_COOOOOOKIE_OM_NOM_NOM
};


OK! That moves the strings correctly. Assuming I don't have a bug in there. The idea's right, but the actual code is from memory and may contain brainfarts.



What you want to have is a bunch of functions, one for each type of data you send. Each of these functions can and should be be tested separately so that when you get to integrating them into the program, the program looks something like



sendString(sock, "Attempting to take a screenshot.");
if (getBitmap("screenshot.bmp"))
{
sendString(sock, "Sending file.");
sendBitmap(sock, "screenshot.bmp");
}


or



receiveString(sock);
std::string command = receiveString(sock);
if (command == "Sending file.")
{
receiveBitmap(sock, "screenshot.bmp");
}
else if (command == "Eating coooooookie! Om! Nom! Nom!")
{
OmNomNom(sock);
}


Which is about a close to foolproof as you can get.



Notes:



There is a bug in the server: int nread = fread(buff, 1, 4096, fp); gets the number of bytes read, but send(clientSocket, buff, sizeof(buff), 0); always tries to send a full buffer regardless of how many bytes were read, so garbage will be sent to the client. Also send can fail and this is not being checked. Always check the return codes. People don't put them there unless they're important.






share|improve this answer





















  • Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
    – programme-zero
    Nov 12 at 19:11












  • Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
    – user4581301
    Nov 12 at 19:31











Your Answer






StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53266068%2fcfile-transfer-with-tcp-protocol%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























1 Answer
1






active

oldest

votes








1 Answer
1






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
1
down vote













TCP is a streaming protocol. It has no concept of messages, so when the server sends "Sending file." there is no separation between the string and the beginning of the file being sent. Everything just goes into the stream one byte after the next and when the network stack decides it's time, usually because a packet has been filled or it's been too long since data was last added, a packet is sent, possibly containing multiple messages.



So



int bytesReceived = recv(sock, buf, 4096, 0);


very likely reads the full 4096 bytes, Attempting to take a screenshot.Sending file. plus the first four thousand-or-so bytes of the bitmap. The client code consumes the string and discards the rest of the buffer.



You need to establish a communication protocol that sits between the the socket and the writing of the file. There are a whole bunch of different ways to handle this. Common tricks for reading strings are




  1. Write the length of the string before writing the string so that the protocol handler knows how many bytes to read ahead of the time


Sender



uint16_t len = str.length(); // size is exactly 16 bits
len = htons(len); // endian is known
int sent = send(sock, (char*)&len, sizeof(len), 0);
// test sent for success (did not fail, sent all the bytes)
sent = send(sock, str.c_str(), len, 0);
// test sent for success (did not fail, sent all the bytes)
// may need to loop here if the string is super long.


Receiver



uint16_t len;
int recd = recv(sock, (char*)&len, sizeof(len), MSG_WAITALL);
// test recd for success (did not fail, read all the bytes)
// MSG_WAITALL will read exactly the right number of bytes or die trying.
len = ntohs(len); // ensure correct endian
std::string msg(len, ' '); // allocate a big enough string
char * msgp = &msg[0]; // or msg.data() if C++17 or better.
// Never seen &msg[0] fail, but this is not guaranteed by C++
while (len) // sometimes you want an extra exit condition here to bail out early
{
recd = recv(sock, msgp, len, 0);
// test recd for success
len -= recd;
msgp += recd;
}



  1. Insert a canary value so that the protocol handler knows when to stop reading. The null terminator works well here. The protocol reads up until it finds the null and preserves the remainder of what's read for later consumption. No code example here because this can be done many, many different ways.

  2. Not using strings and sending integer code messages instead. Eg:




enum messageID
{
TAKING_SCREENSHOT,
SENDING_FILE,
EATING_COOOOOOKIE_OM_NOM_NOM
};


OK! That moves the strings correctly. Assuming I don't have a bug in there. The idea's right, but the actual code is from memory and may contain brainfarts.



What you want to have is a bunch of functions, one for each type of data you send. Each of these functions can and should be be tested separately so that when you get to integrating them into the program, the program looks something like



sendString(sock, "Attempting to take a screenshot.");
if (getBitmap("screenshot.bmp"))
{
sendString(sock, "Sending file.");
sendBitmap(sock, "screenshot.bmp");
}


or



receiveString(sock);
std::string command = receiveString(sock);
if (command == "Sending file.")
{
receiveBitmap(sock, "screenshot.bmp");
}
else if (command == "Eating coooooookie! Om! Nom! Nom!")
{
OmNomNom(sock);
}


Which is about a close to foolproof as you can get.



Notes:



There is a bug in the server: int nread = fread(buff, 1, 4096, fp); gets the number of bytes read, but send(clientSocket, buff, sizeof(buff), 0); always tries to send a full buffer regardless of how many bytes were read, so garbage will be sent to the client. Also send can fail and this is not being checked. Always check the return codes. People don't put them there unless they're important.






share|improve this answer





















  • Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
    – programme-zero
    Nov 12 at 19:11












  • Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
    – user4581301
    Nov 12 at 19:31















up vote
1
down vote













TCP is a streaming protocol. It has no concept of messages, so when the server sends "Sending file." there is no separation between the string and the beginning of the file being sent. Everything just goes into the stream one byte after the next and when the network stack decides it's time, usually because a packet has been filled or it's been too long since data was last added, a packet is sent, possibly containing multiple messages.



So



int bytesReceived = recv(sock, buf, 4096, 0);


very likely reads the full 4096 bytes, Attempting to take a screenshot.Sending file. plus the first four thousand-or-so bytes of the bitmap. The client code consumes the string and discards the rest of the buffer.



You need to establish a communication protocol that sits between the the socket and the writing of the file. There are a whole bunch of different ways to handle this. Common tricks for reading strings are




  1. Write the length of the string before writing the string so that the protocol handler knows how many bytes to read ahead of the time


Sender



uint16_t len = str.length(); // size is exactly 16 bits
len = htons(len); // endian is known
int sent = send(sock, (char*)&len, sizeof(len), 0);
// test sent for success (did not fail, sent all the bytes)
sent = send(sock, str.c_str(), len, 0);
// test sent for success (did not fail, sent all the bytes)
// may need to loop here if the string is super long.


Receiver



uint16_t len;
int recd = recv(sock, (char*)&len, sizeof(len), MSG_WAITALL);
// test recd for success (did not fail, read all the bytes)
// MSG_WAITALL will read exactly the right number of bytes or die trying.
len = ntohs(len); // ensure correct endian
std::string msg(len, ' '); // allocate a big enough string
char * msgp = &msg[0]; // or msg.data() if C++17 or better.
// Never seen &msg[0] fail, but this is not guaranteed by C++
while (len) // sometimes you want an extra exit condition here to bail out early
{
recd = recv(sock, msgp, len, 0);
// test recd for success
len -= recd;
msgp += recd;
}



  1. Insert a canary value so that the protocol handler knows when to stop reading. The null terminator works well here. The protocol reads up until it finds the null and preserves the remainder of what's read for later consumption. No code example here because this can be done many, many different ways.

  2. Not using strings and sending integer code messages instead. Eg:




enum messageID
{
TAKING_SCREENSHOT,
SENDING_FILE,
EATING_COOOOOOKIE_OM_NOM_NOM
};


OK! That moves the strings correctly. Assuming I don't have a bug in there. The idea's right, but the actual code is from memory and may contain brainfarts.



What you want to have is a bunch of functions, one for each type of data you send. Each of these functions can and should be be tested separately so that when you get to integrating them into the program, the program looks something like



sendString(sock, "Attempting to take a screenshot.");
if (getBitmap("screenshot.bmp"))
{
sendString(sock, "Sending file.");
sendBitmap(sock, "screenshot.bmp");
}


or



receiveString(sock);
std::string command = receiveString(sock);
if (command == "Sending file.")
{
receiveBitmap(sock, "screenshot.bmp");
}
else if (command == "Eating coooooookie! Om! Nom! Nom!")
{
OmNomNom(sock);
}


Which is about a close to foolproof as you can get.



Notes:



There is a bug in the server: int nread = fread(buff, 1, 4096, fp); gets the number of bytes read, but send(clientSocket, buff, sizeof(buff), 0); always tries to send a full buffer regardless of how many bytes were read, so garbage will be sent to the client. Also send can fail and this is not being checked. Always check the return codes. People don't put them there unless they're important.






share|improve this answer





















  • Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
    – programme-zero
    Nov 12 at 19:11












  • Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
    – user4581301
    Nov 12 at 19:31













up vote
1
down vote










up vote
1
down vote









TCP is a streaming protocol. It has no concept of messages, so when the server sends "Sending file." there is no separation between the string and the beginning of the file being sent. Everything just goes into the stream one byte after the next and when the network stack decides it's time, usually because a packet has been filled or it's been too long since data was last added, a packet is sent, possibly containing multiple messages.



So



int bytesReceived = recv(sock, buf, 4096, 0);


very likely reads the full 4096 bytes, Attempting to take a screenshot.Sending file. plus the first four thousand-or-so bytes of the bitmap. The client code consumes the string and discards the rest of the buffer.



You need to establish a communication protocol that sits between the the socket and the writing of the file. There are a whole bunch of different ways to handle this. Common tricks for reading strings are




  1. Write the length of the string before writing the string so that the protocol handler knows how many bytes to read ahead of the time


Sender



uint16_t len = str.length(); // size is exactly 16 bits
len = htons(len); // endian is known
int sent = send(sock, (char*)&len, sizeof(len), 0);
// test sent for success (did not fail, sent all the bytes)
sent = send(sock, str.c_str(), len, 0);
// test sent for success (did not fail, sent all the bytes)
// may need to loop here if the string is super long.


Receiver



uint16_t len;
int recd = recv(sock, (char*)&len, sizeof(len), MSG_WAITALL);
// test recd for success (did not fail, read all the bytes)
// MSG_WAITALL will read exactly the right number of bytes or die trying.
len = ntohs(len); // ensure correct endian
std::string msg(len, ' '); // allocate a big enough string
char * msgp = &msg[0]; // or msg.data() if C++17 or better.
// Never seen &msg[0] fail, but this is not guaranteed by C++
while (len) // sometimes you want an extra exit condition here to bail out early
{
recd = recv(sock, msgp, len, 0);
// test recd for success
len -= recd;
msgp += recd;
}



  1. Insert a canary value so that the protocol handler knows when to stop reading. The null terminator works well here. The protocol reads up until it finds the null and preserves the remainder of what's read for later consumption. No code example here because this can be done many, many different ways.

  2. Not using strings and sending integer code messages instead. Eg:




enum messageID
{
TAKING_SCREENSHOT,
SENDING_FILE,
EATING_COOOOOOKIE_OM_NOM_NOM
};


OK! That moves the strings correctly. Assuming I don't have a bug in there. The idea's right, but the actual code is from memory and may contain brainfarts.



What you want to have is a bunch of functions, one for each type of data you send. Each of these functions can and should be be tested separately so that when you get to integrating them into the program, the program looks something like



sendString(sock, "Attempting to take a screenshot.");
if (getBitmap("screenshot.bmp"))
{
sendString(sock, "Sending file.");
sendBitmap(sock, "screenshot.bmp");
}


or



receiveString(sock);
std::string command = receiveString(sock);
if (command == "Sending file.")
{
receiveBitmap(sock, "screenshot.bmp");
}
else if (command == "Eating coooooookie! Om! Nom! Nom!")
{
OmNomNom(sock);
}


Which is about a close to foolproof as you can get.



Notes:



There is a bug in the server: int nread = fread(buff, 1, 4096, fp); gets the number of bytes read, but send(clientSocket, buff, sizeof(buff), 0); always tries to send a full buffer regardless of how many bytes were read, so garbage will be sent to the client. Also send can fail and this is not being checked. Always check the return codes. People don't put them there unless they're important.






share|improve this answer












TCP is a streaming protocol. It has no concept of messages, so when the server sends "Sending file." there is no separation between the string and the beginning of the file being sent. Everything just goes into the stream one byte after the next and when the network stack decides it's time, usually because a packet has been filled or it's been too long since data was last added, a packet is sent, possibly containing multiple messages.



So



int bytesReceived = recv(sock, buf, 4096, 0);


very likely reads the full 4096 bytes, Attempting to take a screenshot.Sending file. plus the first four thousand-or-so bytes of the bitmap. The client code consumes the string and discards the rest of the buffer.



You need to establish a communication protocol that sits between the the socket and the writing of the file. There are a whole bunch of different ways to handle this. Common tricks for reading strings are




  1. Write the length of the string before writing the string so that the protocol handler knows how many bytes to read ahead of the time


Sender



uint16_t len = str.length(); // size is exactly 16 bits
len = htons(len); // endian is known
int sent = send(sock, (char*)&len, sizeof(len), 0);
// test sent for success (did not fail, sent all the bytes)
sent = send(sock, str.c_str(), len, 0);
// test sent for success (did not fail, sent all the bytes)
// may need to loop here if the string is super long.


Receiver



uint16_t len;
int recd = recv(sock, (char*)&len, sizeof(len), MSG_WAITALL);
// test recd for success (did not fail, read all the bytes)
// MSG_WAITALL will read exactly the right number of bytes or die trying.
len = ntohs(len); // ensure correct endian
std::string msg(len, ' '); // allocate a big enough string
char * msgp = &msg[0]; // or msg.data() if C++17 or better.
// Never seen &msg[0] fail, but this is not guaranteed by C++
while (len) // sometimes you want an extra exit condition here to bail out early
{
recd = recv(sock, msgp, len, 0);
// test recd for success
len -= recd;
msgp += recd;
}



  1. Insert a canary value so that the protocol handler knows when to stop reading. The null terminator works well here. The protocol reads up until it finds the null and preserves the remainder of what's read for later consumption. No code example here because this can be done many, many different ways.

  2. Not using strings and sending integer code messages instead. Eg:




enum messageID
{
TAKING_SCREENSHOT,
SENDING_FILE,
EATING_COOOOOOKIE_OM_NOM_NOM
};


OK! That moves the strings correctly. Assuming I don't have a bug in there. The idea's right, but the actual code is from memory and may contain brainfarts.



What you want to have is a bunch of functions, one for each type of data you send. Each of these functions can and should be be tested separately so that when you get to integrating them into the program, the program looks something like



sendString(sock, "Attempting to take a screenshot.");
if (getBitmap("screenshot.bmp"))
{
sendString(sock, "Sending file.");
sendBitmap(sock, "screenshot.bmp");
}


or



receiveString(sock);
std::string command = receiveString(sock);
if (command == "Sending file.")
{
receiveBitmap(sock, "screenshot.bmp");
}
else if (command == "Eating coooooookie! Om! Nom! Nom!")
{
OmNomNom(sock);
}


Which is about a close to foolproof as you can get.



Notes:



There is a bug in the server: int nread = fread(buff, 1, 4096, fp); gets the number of bytes read, but send(clientSocket, buff, sizeof(buff), 0); always tries to send a full buffer regardless of how many bytes were read, so garbage will be sent to the client. Also send can fail and this is not being checked. Always check the return codes. People don't put them there unless they're important.







share|improve this answer












share|improve this answer



share|improve this answer










answered Nov 12 at 18:22









user4581301

19.3k51831




19.3k51831












  • Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
    – programme-zero
    Nov 12 at 19:11












  • Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
    – user4581301
    Nov 12 at 19:31


















  • Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
    – programme-zero
    Nov 12 at 19:11












  • Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
    – user4581301
    Nov 12 at 19:31
















Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
– programme-zero
Nov 12 at 19:11






Thanks for your detailed answer, this is really going to help me. I'm going to try to implement all that and I will give feedback later. EDIT: Just one detail I remembered, is multi threading required to receive the data or it was useless ?
– programme-zero
Nov 12 at 19:11














Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
– user4581301
Nov 12 at 19:31




Depends on what you're doing. If you have other stuff that needs to happen while you're waiting for data to arrive, you're either using threading, polling, or overlapped IO. If you have a single connection and can just sit around and wait for messages, the threading is useless.
– user4581301
Nov 12 at 19:31


















draft saved

draft discarded




















































Thanks for contributing an answer to Stack Overflow!


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.





Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


Please pay close attention to the following guidance:


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53266068%2fcfile-transfer-with-tcp-protocol%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Guess what letter conforming each word

Port of Spain

Run scheduled task as local user group (not BUILTIN)