diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fadbebf..fa9f7133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ HTTP: * Tidy up set payload in http-server-fast * Refine Body::size specification * Newly constructed responses have a 200 OK result +* Refactor file_body for best practices -------------------------------------------------------------------------------- diff --git a/doc/5_08_custom_body.qbk b/doc/5_08_custom_body.qbk index a3bfff7b..baa8f39d 100644 --- a/doc/5_08_custom_body.qbk +++ b/doc/5_08_custom_body.qbk @@ -106,11 +106,17 @@ responses which deliver file contents, and allowing for file uploads. In this example we build the `file_body` type which supports both reading and writing to a file on the file system. -First we declare the type itself, along with the required members: +First we declare the type with its nested types: [example_http_file_body_1] -The `size` function is a simple call to retrieve the file size: +We will start with the definition of the `value_type`. Our strategy +will be to store the open file handle directly in the message +container through the `value_type` field. To use this body it will +be necessary to call `msg.body.open()` with the file first. This +ensures that the file exists throughout the operation and prevent +the race condition where the file is removed from the file system +in between calls. [example_http_file_body_2] diff --git a/example/common/file_body.hpp b/example/common/file_body.hpp index 6da273c0..ba9004b5 100644 --- a/example/common/file_body.hpp +++ b/example/common/file_body.hpp @@ -32,33 +32,6 @@ */ struct file_body { - /** The type of the @ref message::body member. - - Messages declared using `file_body` will have this - type for the body member. We use a path indicating - the location on the file system for which the data - will be read or written. - */ - using value_type = boost::filesystem::path; - - /** Returns the content length of the body in a message. - - This optional static function returns the size of the - body in bytes. It is called from @ref message::size to - return the payload size, and from @ref message::prepare - to automatically set the Content-Length field. If this - function is omitted from a body type, calls to - @ref message::prepare will set the chunked transfer - encoding. - - @param m The message containing a file body to check. - - @return The size of the file in bytes. - */ - static - std::uint64_t - size(value_type const& v); - /** Algorithm for retrieving buffers when serializing. Objects of this type are created during serialization @@ -72,19 +45,121 @@ struct file_body to store incoming buffers representing the body. */ class writer; + + /** The type of the @ref message::body member. + + Messages declared using `file_body` will have this + type for the body member. This rich class interface + allow the file to be opened with the file handle + maintained directly in the object, which is attached + to the message. + */ + class value_type; }; //] //[example_http_file_body_2 -inline -std::uint64_t -file_body:: -size(value_type const& v) +// The body container holds a handle to the file if +// it is open, and we also cache the size upon opening. +// +class file_body::value_type { - return boost::filesystem::file_size(v); -} + friend class reader; + friend class writer; + + FILE* file_ = nullptr; + std::uint64_t size_; + +public: + /** Destructor. + + If the file handle is open, it is closed first. + */ + ~value_type() + { + if(file_) + fclose(file_); + } + + /// Default constructor + value_type() = default; + + /// Move constructor + value_type(value_type&& other) + : file_(other.file_) + , size_(other.size_) + { + other.file_ = nullptr; + } + + /// Move assignment + value_type& operator=(value_type&& other) + { + file_ = other.file_; + size_ = other.size_; + other.file_ = nullptr; + return *this; + } + + /// Returns `true` if the file is open + bool + is_open() const + { + return file_ != nullptr; + } + + /** Open a file for reading or writing. + + @param path The path to the file + + @param mode The open mode used with fopen() + + @param ec Set to the error, if any occurred + */ + void + open(boost::filesystem::path const& path, + char const* mode, beast::error_code& ec) + { + // Attempt to open the file for reading + file_ = fopen(path.string().c_str(), mode); + + if(! file_) + { + // Convert the old-school `errno` into + // an error code using the generic category. + ec = beast::error_code{errno, beast::generic_category()}; + return; + } + + // The file was opened successfully. + // If we are reading, cache the file size. + if(std::string{mode} == "rb") + { + size_ = boost::filesystem::file_size(path, ec); + } + else + { + // This is required by the error_code specification + ec = {}; + } + } + + /** Returns the size of the file. + + Preconditions: + + * The file must be open for binary reading (mode=="rb") + + @return The size of the file if a file is open, else undefined. + */ + std::uint64_t + size() const + { + return size_; + } +}; //] @@ -92,9 +167,8 @@ size(value_type const& v) class file_body::reader { - value_type const& path_; // Path of the file - FILE* file_ = nullptr; // File handle - std::uint64_t remain_ = 0; // The number of unread bytes + FILE* file_; // The file handle + std::uint64_t remain_; // The number of unread bytes char buf_[4096]; // Small buffer for reading public: @@ -112,9 +186,6 @@ public: reader(beast::http::message const& m, beast::error_code& ec); - // Destructor - ~reader(); - // This function is called zero or more times to // retrieve buffers. A return value of `boost::none` // means there are no more buffers. Otherwise, @@ -137,25 +208,14 @@ template file_body::reader:: reader(beast::http::message const& m, beast::error_code& ec) - : path_(m.body) + : file_(m.body.file_) + , remain_(m.body.size()) { - // Attempt to open the file for reading - file_ = fopen(path_.string().c_str(), "rb"); - - if(! file_) - { - // Convert the old-school `errno` into - // an error code using the generic category. - ec = beast::error_code{errno, beast::generic_category()}; - return; - } + // The file must already be open + BOOST_ASSERT(file_ != nullptr); // This is required by the error_code specification ec = {}; - - // The file was opened successfully, get the size - // of the file to know how much we need to read. - remain_ = boost::filesystem::file_size(path_); } // This function is called repeatedly by the serializer to @@ -215,27 +275,13 @@ get(beast::error_code& ec) -> }}; } -// The destructor is always invoked if construction succeeds. -// -inline -file_body::reader:: -~reader() -{ - // Just close the file if its open - if(file_) - fclose(file_); - - // In theory fclose() can fail but how would we handle it? -} - //] //[example_http_file_body_5 class file_body::writer { - value_type const& path_; // A path to the file - FILE* file_ = nullptr; // The file handle + FILE* file_; // The file handle public: // Constructor. @@ -267,12 +313,6 @@ public: // void finish(beast::error_code& ec); - - // Destructor. - // - // Avoid calling anything that might fail here. - // - ~writer(); }; //] @@ -285,20 +325,15 @@ file_body::writer:: writer(beast::http::message& m, boost::optional const& content_length, beast::error_code& ec) - : path_(m.body) + : file_(m.body.file_) { + // We don't do anything with this but a sophisticated + // application might check available space on the device + // to see if there is enough room to store the body. boost::ignore_unused(content_length); - // Attempt to open the file for writing - file_ = fopen(path_.string().c_str(), "wb"); - - if(! file_) - { - // Convert the old-school `errno` into - // an error code using the generic category. - ec = beast::error_code{errno, beast::generic_category()}; - return; - } + // The file must already be open for writing + BOOST_ASSERT(file_ != nullptr); // This is required by the error_code specification ec = {}; @@ -352,19 +387,6 @@ finish(beast::error_code& ec) ec = {}; } -// The destructor is always invoked if construction succeeds -// -inline -file_body::writer:: -~writer() -{ - // Just close the file if its open - if(file_) - fclose(file_); - - // In theory fclose() can fail but how would we handle it? -} - //] #endif diff --git a/example/server-framework/file_service.hpp b/example/server-framework/file_service.hpp index 36571529..96079081 100644 --- a/example/server-framework/file_service.hpp +++ b/example/server-framework/file_service.hpp @@ -129,16 +129,20 @@ public: // Calculate full path from root boost::filesystem::path full_path = root_ / rel_path; - // Make sure the file is there - if(boost::filesystem::exists(full_path)) + beast::error_code ec; + auto res = get(req, full_path, ec); + + if(ec == beast::errc::no_such_file_or_directory) { - // Send the file - send(get(req, full_path)); + send(not_found(req, rel_path)); + } + else if(ec) + { + send(server_error(req, rel_path, ec)); } else { - // Send a Not Found result - send(not_found(req, rel_path)); + send(std::move(*res)); } // Indicate that we handled the request @@ -156,16 +160,20 @@ public: // Calculate full path from root boost::filesystem::path full_path = root_ / rel_path; - // Make sure the file is there - if(boost::filesystem::exists(full_path)) + beast::error_code ec; + auto res = head(req, full_path, ec); + + if(ec == beast::errc::no_such_file_or_directory) { - // Send a HEAD response - send(head(req, full_path)); + send(not_found(req, rel_path)); + } + else if(ec) + { + send(server_error(req, rel_path, ec)); } else { - // Send a Not Found result - send(not_found(req, rel_path)); + send(std::move(*res)); } // Indicate that we handled the request @@ -202,44 +210,66 @@ private: return res; } + // Return an HTTP Server Error + // + template + beast::http::response + server_error( + beast::http::request const& req, + boost::filesystem::path const& rel_path, + error_code const& ec) const + { + boost::ignore_unused(rel_path); + beast::http::response res; + res.version = req.version; + res.result(beast::http::status::internal_server_error); + res.set(beast::http::field::server, server_); + res.set(beast::http::field::content_type, "text/html"); + res.body = "Error: " + ec.message(); + res.prepare_payload(); + return res; + } + // Return a file response to an HTTP GET request // template - beast::http::response + boost::optional> get( beast::http::request const& req, - boost::filesystem::path const& full_path) const + boost::filesystem::path const& full_path, + beast::error_code& ec) const { beast::http::response res; res.version = req.version; - res.result(beast::http::status::ok); res.set(beast::http::field::server, server_); res.set(beast::http::field::content_type, mime_type(full_path)); - res.body = full_path; - res.prepare_payload(); + res.body.open(full_path, "rb", ec); + if(ec) + return boost::none; + res.set(beast::http::field::content_length, res.body.size()); return res; } // Return a response to an HTTP HEAD request // template - beast::http::response + boost::optional> head( beast::http::request const& req, - boost::filesystem::path const& full_path) const + boost::filesystem::path const& full_path, + beast::error_code& ec) const { beast::http::response res; res.version = req.version; - res.result(beast::http::status::ok); res.set(beast::http::field::server, server_); res.set(beast::http::field::content_type, mime_type(full_path)); - // Set the Content-Length field manually. We don't have a body, - // but this is a response to a HEAD request so we include the - // content length anyway. - // - res.set(beast::http::field::content_length, file_body::size(full_path)); - + // Use a manual file body here + file_body::value_type body; + body.open(full_path, "rb", ec); + if(ec) + return boost::none; + res.set(beast::http::field::content_length, body.size()); return res; } }; diff --git a/test/http/doc_examples.cpp b/test/http/doc_examples.cpp index 5f29c58e..e800a899 100644 --- a/test/http/doc_examples.cpp +++ b/test/http/doc_examples.cpp @@ -311,18 +311,24 @@ public: BEAST_EXPECTS(p0.get().method() == verb::put, p0.get().method_string()); { + error_code ec; request_parser p{std::move(p0)}; - p.get().body = path; + p.get().body.open(path, "wb", ec); + if(ec) + BOOST_THROW_EXCEPTION(system_error{ec}); read(c.server, b, p); } } { + error_code ec; response res; res.version = 11; res.result(status::ok); res.insert(field::server, "test"); - res.body = path; - res.prepare_payload(); + res.body.open(path, "rb", ec); + if(ec) + BOOST_THROW_EXCEPTION(system_error{ec}); + res.set(field::content_length, res.body.size()); write(c.server, res); } {