Skip to content

GitLab

Commit 32d1e91a authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[5451] Addressed review comments.

parent 4c23ca08
Hide whitespace changes
Inline Side-by-side
src/bin/agent/ca_response_creator.h
View file @ 32d1e91a
......@@ -59,6 +59,10 @@ private:
/// @brief Creates unfinalized stock HTTP response.
///
/// The unfinilized response is the response that can't be sent over the
/// wire until @c finalize() is called, which commits the contents of the
/// message body.
///
/// @param request Pointer to an object representing HTTP request.
/// @param status_code Status code of the response.
/// @return Pointer to an @ref isc::http::HttpResponseJson object
......
src/lib/http/client.cc
View file @ 32d1e91a
......@@ -25,9 +25,6 @@ using namespace http;
namespace {
/// @brief Default request timeout of 10s.
const long REQUEST_TIMEOUT = 10000;
/// @brief TCP socket callback function type.
typedef boost::function<void(boost::system::error_code ec, size_t length)>
SocketCallbackFunction;
......@@ -119,9 +116,6 @@ public:
void doTransaction(const HttpRequestPtr& request, const HttpResponsePtr& response,
const long request_timeout, const HttpClient::RequestHandler& callback);
/// @brief Closes connection and removes it from the connection pool.
void stop();
/// @brief Closes the socket and cancels the request timer.
void close();
......@@ -464,12 +458,6 @@ Connection::doTransaction(const HttpRequestPtr& request,
}
}
void
Connection::stop() {
ConnectionPoolPtr conn_pool = conn_pool_.lock();
conn_pool->closeConnection(url_);
}
void
Connection::close() {
timer_.cancel();
......@@ -668,17 +656,8 @@ HttpClient::HttpClient(IOService& io_service)
void
HttpClient::asyncSendRequest(const Url& url, const HttpRequestPtr& request,
const HttpResponsePtr& response,
const HttpClient::RequestHandler& callback) {
asyncSendRequest(url, request, response,
HttpClient::RequestTimeout(REQUEST_TIMEOUT),
callback);
}
void
HttpClient::asyncSendRequest(const Url& url, const HttpRequestPtr& request,
const HttpResponsePtr& response,
const HttpClient::RequestTimeout& request_timeout,
const HttpClient::RequestHandler& callback) {
const HttpClient::RequestHandler& callback,
const HttpClient::RequestTimeout& request_timeout) {
if (!url.isValid()) {
isc_throw(HttpClientError, "invalid URL specified for the HTTP client");
}
......
src/lib/http/client.h
View file @ 32d1e91a
......@@ -29,6 +29,40 @@ public:
class HttpClientImpl;
/// @brief HTTP client class.
///
/// This class implements an asynchronous HTTP client. The caller can schedule
/// transmission of the HTTP request using @ref HttpClient::asyncSendRequest
/// method. The caller specifies target URL for each request. The caller also
/// specifies a pointer to the @ref HttpRequest or derived class, holding a
/// request that should be transmitted to the destination. Such request must
/// be finalized, i.e. @ref HttpRequest::finalize method must be called prior
/// to sending it. The caller must also provide a pointer to the
/// @ref HttpResponse object or an object derived from it. The type of the
/// response object must match the expected content type to be returned in the
/// server's response. The last argument specified in this call is the pointer
/// to the callback function, which should be launched when the response is
/// received, an error occurs or when a timeout in the transmission is
/// signalled.
///
/// The HTTP client supports multiple simultaneous and persistent connections
/// with different destinations. The client determines if the connection is
/// persistent by looking into the Connection header and HTTP version of the
/// request. If the connection should be persistent the client doesn't
/// close the connection after sending a request and receiving a response from
/// the server. If the client is provided with the request to be sent to the
/// particular destination, but there is an ongoing communication with this
/// destination, e.g. as a result of sending previous request, the new
/// request is queued in the FIFO queue. When the previous request completes,
/// the next request in the queue for the particular URL will be initiated.
///
/// The client tests the persistent connection for usability before sending
/// a request by trying to read from the socket (with message peeking). If
/// the socket is usable the client uses it to transmit the request.
///
/// All errors are reported to the caller via the callback function supplied
/// to the @ref HttpClient::asyncSendRequest. The IO errors are communicated
/// via the @c boost::system::error code value. The response parsing errors
/// are returned via the 3rd parameter of the callback.
class HttpClient {
public:
......@@ -55,51 +89,54 @@ public:
/// @brief Queues new asynchronous HTTP request.
///
/// The client creates one connection for the specified URL. If the connection
/// with the particular destination already exists, it will be re-used for the
/// new transaction scheduled with this call. If another transaction is still
/// in progress, the new transaction is queued. The queued transactions are
/// started in the FIFO order one after another. If the connection is idle or
/// the connection doesn't exist, the new transaction is started immediatelly.
/// The client creates one connection for the specified URL. If the
/// connection with the particular destination already exists, it will be
/// re-used for the new transaction scheduled with this call. If another
/// transaction is still in progress, the new transaction is queued. The
/// queued transactions are started in the FIFO order one after another. If
/// the connection is idle or the connection doesn't exist, the new
/// transaction is started immediatelly.
///
/// The existing connection is tested before it is used for the new transaction
/// by attempting to read (with message peeking) from the open TCP socket. If the
/// read attempt is successful, the client will transmit the HTTP request to
/// the server using this connection. It is possible that the server closes the
/// connection between the connection test and sending the request. In such case,
/// an error will be returned and the caller will need to try re-sending the
/// request.
/// The existing connection is tested before it is used for the new
/// transaction by attempting to read (with message peeking) from the open
/// TCP socket. If the read attempt is successful, the client will transmit
/// the HTTP request to the server using this connection. It is possible
/// that the server closes the connection between the connection test and
/// sending the request. In such case, an error will be returned and the
/// caller will need to try re-sending the request.
///
/// If the connection test fails, the client will close the socket and reconnect
/// to the server prior to sending the request.
/// If the connection test fails, the client will close the socket and
/// reconnect to the server prior to sending the request.
///
/// Pointers to the request and response objects are provided as arguments of
/// this method. These pointers should have appropriate types derived from the
/// @ref HttpRequest and @ref HttpResponse classes. For example, if the request
/// has content type "application/json", a pointer to the
/// Pointers to the request and response objects are provided as arguments
/// of this method. These pointers should have appropriate types derived
/// from the @ref HttpRequest and @ref HttpResponse classes. For example,
/// if the request has content type "application/json", a pointer to the
/// @ref HttpResponseJson should be passed. In this case, the response type
/// should be @ref HttpResponseJson. These types are used to validate both the
/// request provided by the caller and the response received from the server.
/// should be @ref HttpResponseJson. These types are used to validate both
/// the request provided by the caller and the response received from the
/// server.
///
/// The callback function provided by the caller is invoked when the transaction
/// terminates, i.e. when the server has responded or when an error occured. The
/// callback is expected to be exception safe, but the client internally guards
/// against exceptions thrown by the callback.
/// The callback function provided by the caller is invoked when the
/// transaction terminates, i.e. when the server has responded or when an
/// error occured. The callback is expected to be exception safe, but the
/// client internally guards against exceptions thrown by the callback.
///
/// The first argument of the callback indicates an IO error during communication
/// with the server. If the communication is successful the error code of 0 is
/// returned. However, in this case it is still possible that the transaction is
/// unsuccessful due to HTTP response parsing error, e.g. invalid content type,
/// malformed response etc. Such errors are indicated via third argument.
/// The first argument of the callback indicates an IO error during
/// communication with the server. If the communication is successful the
/// error code of 0 is returned. However, in this case it is still possible
/// that the transaction is unsuccessful due to HTTP response parsing error,
/// e.g. invalid content type, malformed response etc. Such errors are
/// indicated via third argument.
///
/// If message parsing was successful the second argument of the callback contains
/// a pointer to the parsed response (the same pointer as provided by the caller
/// as the argument). If parsing was unsuccessful, the null pointer is returned.
/// If message parsing was successful the second argument of the callback
/// contains a pointer to the parsed response (the same pointer as provided
///by the caller as the argument). If parsing was unsuccessful, the null
/// pointer is returned.
///
/// The default timeout for the transaction is set to 10 seconds (10 000 ms). This
/// variant of the method doesn't allow for specifying a custom timeout. If the
/// timeout occurs, the callback is invoked with the error code of
/// @c boost::asio::error::timed_out.
/// The default timeout for the transaction is set to 10 seconds
/// (10 000 ms). If the timeout occurs, the callback is invoked with the
//// error code of @c boost::asio::error::timed_out.
///
/// @param url URL where the request should be send.
/// @param request Pointer to the object holding a request.
......@@ -110,22 +147,9 @@ public:
void asyncSendRequest(const Url& url,
const HttpRequestPtr& request,
const HttpResponsePtr& response,
const RequestHandler& callback);
/// @brief Queues new asynchronous HTTP request with timeout.
///
/// @param url URL where the request should be send.
/// @param request Pointer to the object holding a request.
/// @param response Pointer to the object where response should be stored.
/// @param request_timeout Timeout for the transaction in milliseconds.
/// @param callback Pointer to the user callback function.
///
/// @throw HttpClientError If invalid arguments were provided.
void asyncSendRequest(const Url& url,
const HttpRequestPtr& request,
const HttpResponsePtr& response,
const RequestTimeout& request_timeout,
const RequestHandler& callback);
const RequestHandler& callback,
const RequestTimeout& request_timeout =
RequestTimeout(10000));
/// @brief Closes all connections.
void stop();
......
src/lib/http/http_message.cc
View file @ 32d1e91a
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -56,20 +56,6 @@ HttpMessage::getHttpVersion() const {
HttpHeaderPtr
HttpMessage::getHeader(const std::string& header_name) const {
HttpHeaderPtr http_header = getHeaderSafe(header_name);
// No such header.
if (!http_header) {
isc_throw(HttpMessageNonExistingHeader, header_name << " HTTP header"
" not found in the request");
}
// Header found.
return (http_header);
}
HttpHeaderPtr
HttpMessage::getHeaderSafe(const std::string& header_name) const {
checkCreated();
HttpHeader hdr(header_name);
......@@ -78,8 +64,8 @@ HttpMessage::getHeaderSafe(const std::string& header_name) const {
return (header_it->second);
}
// Header not found. Return null pointer.
return (HttpHeaderPtr());
isc_throw(HttpMessageNonExistingHeader, header_name << " HTTP header"
" not found in the request");
}
std::string
......
src/lib/http/http_message.h
View file @ 32d1e91a
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -162,19 +162,6 @@ public:
/// @throw HttpMessageError if the request hasn't been created.
HttpHeaderPtr getHeader(const std::string& header_name) const;
/// @brief Returns object encapsulating HTTP header.
///
/// This variant doesn't throw an exception if the header doesn't exist.
/// It will throw if the request hasn't been created using @c create()
/// method.
///
/// @param header_name HTTP header name.
///
/// @return Pointer to the specified header, or null if such header doesn't
/// exist.
/// @throw HttpMessageError if the request hasn't been created.
HttpHeaderPtr getHeaderSafe(const std::string& header_name) const;
/// @brief Returns a value of the specified HTTP header.
///
/// @param header_name Name of the HTTP header.
......
src/lib/http/http_message_parser_base.cc
View file @ 32d1e91a
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -143,7 +143,7 @@ HttpMessageParserBase::getNextFromBuffer() {
// NEED_MORE_DATA_EVT it indicates that the caller hasn't provided
// the data.
if (ev == NEED_MORE_DATA_EVT) {
isc_throw(HttpMessageParserBaseError,
isc_throw(HttpParseError,
"HTTP request parser requires new data to progress, but no data"
" have been provided. The transaction is aborted to avoid"
" a deadlock. This is a Kea HTTP server logic error!");
......@@ -155,7 +155,7 @@ HttpMessageParserBase::getNextFromBuffer() {
// There is no more data so it is really not possible that we're
// at MORE_DATA_PROVIDED_EVT.
if (ev == MORE_DATA_PROVIDED_EVT) {
isc_throw(HttpMessageParserBaseError,
isc_throw(HttpParseError,
"HTTP server state indicates that new data have been"
" provided to be parsed, but the transaction buffer"
" contains no new data. This is a Kea HTTP server logic"
......@@ -174,7 +174,7 @@ HttpMessageParserBase::getNextFromBuffer() {
void
HttpMessageParserBase::invalidEventError(const std::string& handler_name,
const unsigned int event) {
isc_throw(HttpMessageParserBaseError, handler_name << ": "
isc_throw(HttpParseError, handler_name << ": "
<< " invalid event " << getEventLabel(static_cast<int>(event)));
}
......
src/lib/http/http_message_parser_base.h
View file @ 32d1e91a
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -21,9 +21,9 @@ namespace http {
/// has occurred.
///
/// The most common errors are due to receiving malformed requests.
class HttpMessageParserBaseError : public Exception {
class HttpParseError : public Exception {
public:
HttpMessageParserBaseError(const char* file, size_t line, const char* what) :
HttpParseError(const char* file, size_t line, const char* what) :
isc::Exception(file, line, what) { };
};
......
src/lib/http/request.cc
View file @ 32d1e91a
......@@ -169,7 +169,15 @@ HttpRequest::toString() const {
bool
HttpRequest::isPersistent() const {
HttpHeaderPtr conn = getHeaderSafe("connection");
HttpHeaderPtr conn;
try {
conn = getHeader("connection");
} catch (...) {
// If there is an exception, it means that the header was not found.
}
std::string conn_value;
if (conn) {
conn_value = conn->getLowerCaseValue();
......
src/lib/http/request_parser.h
View file @ 32d1e91a
// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -14,16 +14,6 @@
namespace isc {
namespace http {
/// @brief Exception thrown when an error during parsing HTTP request
/// has occurred.
///
/// The most common errors are due to receiving malformed requests.
class HttpRequestParserError : public HttpMessageParserBaseError {
public:
HttpRequestParserError(const char* file, size_t line, const char* what) :
HttpMessageParserBaseError(file, line, what) { };
};
class HttpRequestParser;
/// @brief Pointer to the @ref HttpRequestParser.
......
src/lib/http/response_json.h
View file @ 32d1e91a
// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -54,7 +54,9 @@ public:
/// @brief Completes creation of the HTTP response.
///
/// This method marks the response as finalized. The JSON structure is
/// created and can be used to retrieve the parsed data.
/// created and can be used to retrieve the parsed data. If this is the
/// outbound message, it can be transmitted over the wire as the body
/// for the message is now committed.
virtual void finalize();
/// @brief Reset the state of the object.
......
src/lib/http/response_parser.h
View file @ 32d1e91a
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -14,16 +14,6 @@
namespace isc {
namespace http {
/// @brief Exception thrown when an error during parsing HTTP response
/// has occurred.
///
/// The most common errors are due to receiving malformed response.
class HttpResponseParserError : public HttpMessageParserBaseError {
public:
HttpResponseParserError(const char* file, size_t line, const char* what) :
HttpMessageParserBaseError(file, line, what) { };
};
class HttpResponseParser;
/// @brief Pointer to the @ref HttpResponseParser.
......
src/lib/http/tests/post_request_json_unittests.cc
View file @ 32d1e91a
// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -184,14 +184,14 @@ TEST_F(PostHttpRequestJsonTest, clientRequest) {
// Commit and validate the data.
ASSERT_NO_THROW(request_.finalize());
std::ostringstream expected_response;
expected_response << "POST /isc/org HTTP/1.0\r\n"
std::ostringstream expected_request_text;
expected_request_text << "POST /isc/org HTTP/1.0\r\n"
"Content-Length: " << json->str().size() << "\r\n"
"Content-Type: application/json\r\n"
"\r\n"
<< json->str();
EXPECT_EQ(expected_response.str(), request_.toString());
EXPECT_EQ(expected_request_text.str(), request_.toString());
}
}
src/lib/http/tests/server_client_unittests.cc
View file @ 32d1e91a
......@@ -1217,7 +1217,6 @@ TEST_F(HttpClientTest, clientRequestTimeout) {
PostHttpRequestJsonPtr request1 = createRequest("partial-response", true);
HttpResponseJsonPtr response1(new HttpResponseJson());
ASSERT_NO_THROW(client.asyncSendRequest(url, request1, response1,
HttpClient::RequestTimeout(100),
[this, &cb_num](const boost::system::error_code& ec,
const HttpResponsePtr& response,
const std::string&) {
......@@ -1230,7 +1229,7 @@ TEST_F(HttpClientTest, clientRequestTimeout) {
EXPECT_TRUE(ec.value() == boost::asio::error::timed_out);
// There should be no response returned.
EXPECT_FALSE(response);
}));
}, HttpClient::RequestTimeout(100)));
// Create another request after the timeout. It should be handled ok.
PostHttpRequestJsonPtr request2 = createRequest("sequence", 1);
......
src/lib/http/url.cc
View file @ 32d1e91a
......@@ -199,6 +199,9 @@ Url::parse() {
// If there is anything left in the URL, we consider it a path.
if (offset2 != std::string::npos) {
path_ = url_.substr(offset2);
if (path_.empty()) {
path_ = "/";
}
}
valid_ = true;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment