read_until might read beyond the delimiter (therefore request_buf.size() can be more than siz). This is a conceptual problem when you implement save because you read data_size bytes from the socket, which ignores any data already in request_buf
These things are code smells:
if (output_file.tellp() == (std::fstream::pos_type)(std::streamsize)filesize) {
(never use C-style casts). And
return __LINE__; // huh? just `true` then
And
buf.empty();
(That has no effect whatsoever).
I present here three versions:
- First Cleanup
- Simplify (using
tcp::iostream)
- Simplify! (assuming more things about the request format)
First Cleanup
Here's a reasonable cleanup:
Live On Coliru
#include <boost/asio.hpp>
#include <boost/array.hpp>
#include <iostream>
#include <fstream>
namespace ba = boost::asio;
using ba::ip::tcp;
struct Conf {
int def_port = 6767;
} s_config;
struct Request {
std::string command;
std::string parameter;
std::size_t data_size = 0;
std::string get_filename() const {
// cut filename from path - TODO use boost::filesystem::path instead
return parameter.substr(parameter.find_last_of('\\') + 1);
}
friend std::istream& operator>>(std::istream& is, Request& req) {
return is >> req.command >> req.parameter >> req.data_size;
}
};
struct Sync {
bool start_server();
bool save(Request const& req, boost::asio::streambuf& request_buf);
ba::io_service& io_service;
tcp::socket socket{ io_service };
Conf const *conf = &s_config;
};
bool Sync::start_server() {
boost::asio::streambuf request_buf;
boost::system::error_code error;
try {
tcp::acceptor acceptor(io_service, tcp::endpoint(tcp::v4(), conf->def_port));
acceptor.accept(socket); // socket is a member of class Sync
while (true) {
error.clear();
std::string req_txt;
{
char const* delim = "\n\n";
size_t siz = boost::asio::read_until(socket, request_buf, delim, error);
// correct for actual request siz
auto b = buffers_begin(request_buf.data()),
e = buffers_end(request_buf.data());
auto where = std::search(b, e, delim, delim+strlen(delim));
siz = where==e
? std::distance(b,e)
: std::distance(b,where)+strlen(delim);
std::copy_n(b, siz, back_inserter(req_txt));
request_buf.consume(siz); // consume only the request text bits from the buffer
}
std::cout << "request size:" << req_txt.size() << "\n";
std::cout << "Request text: '" << req_txt << "'\n";
Request req;
{
std::istringstream request_stream(req_txt);
request_stream.exceptions(std::ios::failbit);
request_stream >> req;
}
save(req, request_buf); // parameter is filename
}
} catch (std::exception &e) {
std::cerr << "Error parsing request: " << e.what() << std::endl;
}
return false;
}
bool Sync::save(Request const& req, boost::asio::streambuf& request_buf) {
auto filesize = req.data_size;
std::cout << "filesize is: " << filesize << "\n";
{
std::ofstream output_file(req.get_filename(), std::ios::binary);
if (!output_file) {
std::cout << "failed to open " << req.get_filename() << std::endl;
return true;
}
// deplete request_buf
if (request_buf.size()) {
if (request_buf.size() < filesize)
{
filesize -= request_buf.size();
output_file << &request_buf;
}
else {
// copy only filesize already available bytes
std::copy_n(std::istreambuf_iterator<char>(&request_buf), filesize,
std::ostreambuf_iterator<char>(output_file));
filesize = 0;
}
}
while (filesize) {
boost::array<char, 1024> buf;
boost::system::error_code error;
std::streamsize len = socket.read_some(boost::asio::buffer(buf), error);
if (len > 0)
{
output_file.write(buf.c_array(), len);
filesize -= len;
}
if (error) {
socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, error); // ignore error
socket.close(error);
break; // an error occured
}
}
} // closes output_file
return false;
}
int main() {
ba::io_service svc;
Sync s{svc};
s.start_server();
svc.run();
}
Prints with a client like echo -ne "save test.txt 12\n\nHello world\n" | netcat 127.0.0.1 6767:
request size:18
Request text: 'save test.txt 12
'
filesize is: 12
request size:1
Request text: '
'
Error parsing request: basic_ios::clear: iostream error
SIMPLIFY
However, since everything is synchronous, why not just use tcp::iostream socket;. That would make start_server look like this:
tcp::acceptor acceptor(io_service, tcp::endpoint(tcp::v4(), conf->def_port));
acceptor.accept(*socket.rdbuf());
while (socket) {
std::string req_txt, line;
while (getline(socket, line) && !line.empty()) {
req_txt += line + "\n";
}
std::cout << "request size:" << req_txt.size() << "\n";
std::cout << "Request text: '" << req_txt << "'\n";
Request req;
if (std::istringstream(req_txt) >> req)
save(req);
}
And save even simpler:
void Sync::save(Request const& req) {
char buf[1024];
size_t remain = req.data_size, n = 0;
for (std::ofstream of(req.get_filename(), std::ios::binary);
socket.read(buf, std::min(sizeof(buf), remain)), (n = socket.gcount());
remain -= n)
{
if (!of.write(buf, n))
break;
}
}
See it Live On Coliru
When tested with
for f in test{a..z}.txt; do (echo -ne "save $f 12\n\nHello world\n"); done | netcat 127.0.0.1 6767
that prints:
request size:18
Request text: 'save testa.txt 12
'
request size:18
Request text: 'save testb.txt 12
'
[... snip ...]
request size:18
Request text: 'save testz.txt 12
'
request size:0
Request text: ''
Even Simpler
If you know that the request is a single line, or whitespace is not significant:
struct Sync {
void run_server();
void save(Request const& req);
private:
Conf const *conf = &s_config;
tcp::iostream socket;
};
void Sync::run_server() {
ba::io_service io_service;
tcp::acceptor acceptor(io_service, tcp::endpoint(tcp::v4(), conf->def_port));
acceptor.accept(*socket.rdbuf());
for (Request req; socket >> std::noskipws >> req; std::cout << req << " handled\n")
save(req);
}
void Sync::save(Request const& req) {
char buf[1024];
size_t remain = req.data_size, n = 0;
for (std::ofstream of(req.get_filename(), std::ios::binary);
socket.read(buf, std::min(sizeof(buf), remain)), (n = socket.gcount());
remain -= n)
{
if (!of.write(buf, n)) break;
}
}
int main() {
Sync().run_server();
}
That's the entire program in ~33 lines of code. See it Live On Coliru, printing:
Request {"save" "testa.txt"} handled
Request {"save" "testb.txt"} handled
Request {"save" "testc.txt"} handled
[... snip ...]
Request {"save" "testy.txt"} handled
Request {"save" "testz.txt"} handled
