Skip to content

example: send response content-length as part of http header from server #808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deanberris merged 3 commits into from
Jan 26, 2018
Merged

example: send response content-length as part of http header from server #808

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18cc913
example: send response content-length as part of http header from ser…
carun Jan 13, 2018
7d70242
fix build warnings with member initialization order in async_message.hpp
carun Jan 13, 2018
7a0cbe7
Implemented review comments.
carun Jan 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion boost/network/protocol/http/message/async_message.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ struct async_message {
destination_;
mutable boost::shared_future<std::uint16_t> status_;
mutable boost::shared_future<headers_container_type> headers_;
mutable boost::optional<headers_container_type> retrieved_headers_;
mutable headers_container_type added_headers;
mutable std::set<string_type> removed_headers;
mutable boost::shared_future<string_type> body_;
mutable boost::optional<headers_container_type> retrieved_headers_;

friend struct boost::network::http::impl::ready_wrapper<Tag>;
};
Expand Down
18 changes: 12 additions & 6 deletions libs/network/example/http/async_server_file_upload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ struct connection_handler {
void operator()(server::request const& req, const server::connection_ptr& conn) {
static std::map<std::string, std::string> headers = {
{"Connection","close"},
{"Content-Length", "0"},
Copy link
Copy Markdown
Member

@deanberris deanberris Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this?

Copy link
Copy Markdown
Author

@carun carun Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the reason for #681

Copy link
Copy Markdown
Member

@deanberris deanberris Jan 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand now, that you want to always define this for every new response. That makes sense.

Copy link
Copy Markdown
Member

@deanberris deanberris Jan 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this map really ought to not be static, unless you're going to make copies of it for every instance of this handler -- note that there could be multiple threads accessing this map, and updating it on the fly will cause all sorts of threading issues. Aside from that, the other changes look good.

{"Content-Type", "text/plain"}
};

Expand All @@ -206,24 +207,29 @@ struct connection_handler {
// Wait until the data transfer is done by the IO threads
uploader->wait_for_completion();

// Respond to the client
conn->set_status(server::connection::ok);
conn->set_headers(headers);
auto end = std::chrono::high_resolution_clock::now();
std::chrono::duration<double, std::milli> diff = end - start;
std::ostringstream stm;
stm << "Took " << diff.count() << " milliseconds for the transfer." << std::endl;
conn->write(stm.str());
auto body = stm.str();
// Respond to the client
headers["Content-Length"] = std::to_string(body.size());
conn->set_status(server::connection::ok);
conn->set_headers(headers);
conn->write(body);
} catch (const file_uploader_exception& e) {
const std::string err = e.what();
headers["Content-Length"] = std::to_string(err.size());
conn->set_status(server::connection::bad_request);
conn->set_headers(headers);
const std::string err = e.what();
conn->write(err);
}
} else {
static std::string body("Only path allowed is /upload");
Copy link
Copy Markdown
Member

@deanberris deanberris Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually a bad idea to have function-local static objects with non-trivial destructors. Consider just turning this into a character array that's also constexpr:

static constexpr char body[] = "Only path allowed is /upload";

Copy link
Copy Markdown
Author

@carun carun Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will fix it.

headers["Content-Length"] = std::to_string(body.size());
conn->set_status(server::connection::bad_request);
conn->set_headers(headers);
conn->write("Only path allowed is /upload.");
conn->write(body);
}
}
};
Expand Down
7 changes: 5 additions & 2 deletions libs/network/example/http/hello_world_async_server_with_work_queue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,16 @@ void process_request(work_queue& queue) {
// some heavy work!
std::this_thread::sleep_for(std::chrono::seconds(10));

std::map<std::string, std::string> headers = {
static std::map<std::string, std::string> headers = {
Copy link
Copy Markdown
Member

@deanberris deanberris Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be static?

Copy link
Copy Markdown
Author

@carun carun Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so that the map doesn't get created every time the function is called.

{"Content-Length", "0"},
{"Content-Type", "text/plain"},
};

std::string body("Hello, world!");
headers["Content-Length"] = std::to_string(body.size());
request->conn->set_status(server::connection::ok);
request->conn->set_headers(headers);
request->conn->write("Hello, world!");
request->conn->write(body);
}

std::this_thread::sleep_for(std::chrono::microseconds(1000));
Expand Down
8 changes: 6 additions & 2 deletions libs/network/example/http/hello_world_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,17 @@ struct hello_world {
std::ostringstream data;
data << "Hello, " << ip << ':' << port << '!';

std::map<std::string, std::string> headers = {
static std::map<std::string, std::string> headers = {
Copy link
Copy Markdown
Member

@deanberris deanberris Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, why static?

Copy link
Copy Markdown
Author

@carun carun Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.. if it is not required, I can remove it.

{"Content-Length", "0"},
{"Content-Type", "text/plain"},
};

auto body = data.str();
headers["Content-Length"] = std::to_string(body.size());

connection->set_status(server::connection::ok);
connection->set_headers(headers);
connection->write(data.str());
connection->write(body);
}
};

Expand Down