* ClientImpl: Connection=close header control moved from process_request to send_

* Connection=close header control moved from send_ to handle_request

* SSLClient::connect_with_proxy error handling improved

* to_string definition added for Error::ProxyConnection

* Comment improvement

---------

Co-authored-by: akinelden <akin.elden@gmail.com>
This commit is contained in:
Akın ELDEN 2023-07-29 19:09:25 +03:00 committed by GitHub
parent ec87b04aff
commit 961a9379d5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -950,6 +950,7 @@ enum class Error {
UnsupportedMultipartBoundaryChars,
Compression,
ConnectionTimeout,
ProxyConnection,
// For internal use only
SSLPeerCouldBeClosed_,
@ -1888,6 +1889,7 @@ inline std::string to_string(const Error error) {
return "Unsupported HTTP multipart boundary characters";
case Error::Compression: return "Compression failed";
case Error::ConnectionTimeout: return "Connection timed out";
case Error::ProxyConnection: return "Proxy connection failed";
case Error::Unknown: return "Unknown";
default: break;
}
@ -6748,6 +6750,21 @@ inline bool ClientImpl::handle_request(Stream &strm, Request &req,
if (!ret) { return false; }
if (res.get_header_value("Connection") == "close" ||
(res.version == "HTTP/1.0" && res.reason != "Connection established")) {
// TODO this requires a not-entirely-obvious chain of calls to be correct
// for this to be safe.
// This is safe to call because handle_request is only called by send_
// which locks the request mutex during the process. It would be a bug
// to call it from a different thread since it's a thread-safety issue
// to do these things to the socket if another thread is using the socket.
std::lock_guard<std::mutex> guard(socket_mutex_);
shutdown_ssl(socket_, true);
shutdown_socket(socket_);
close_socket(socket_);
}
if (300 < res.status && res.status < 400 && follow_location_) {
req = req_save;
ret = redirect(req, res, error);
@ -7162,24 +7179,6 @@ inline bool ClientImpl::process_request(Stream &strm, Request &req,
}
}
if (res.get_header_value("Connection") == "close" ||
(res.version == "HTTP/1.0" && res.reason != "Connection established")) {
// TODO this requires a not-entirely-obvious chain of calls to be correct
// for this to be safe. Maybe a code refactor (such as moving this out to
// the send function and getting rid of the recursiveness of the mutex)
// could make this more obvious.
// This is safe to call because process_request is only called by
// handle_request which is only called by send, which locks the request
// mutex during the process. It would be a bug to call it from a different
// thread since it's a thread-safety issue to do these things to the socket
// if another thread is using the socket.
std::lock_guard<std::mutex> guard(socket_mutex_);
shutdown_ssl(socket_, true);
shutdown_socket(socket_);
close_socket(socket_);
}
// Log
if (logger_) { logger_(req, res); }
@ -8287,14 +8286,14 @@ inline bool SSLClient::create_and_connect_socket(Socket &socket, Error &error) {
inline bool SSLClient::connect_with_proxy(Socket &socket, Response &res,
bool &success, Error &error) {
success = true;
Response res2;
Response proxy_res;
if (!detail::process_client_socket(
socket.sock, read_timeout_sec_, read_timeout_usec_,
write_timeout_sec_, write_timeout_usec_, [&](Stream &strm) {
Request req2;
req2.method = "CONNECT";
req2.path = host_and_port_;
return process_request(strm, req2, res2, false, error);
return process_request(strm, req2, proxy_res, false, error);
})) {
// Thread-safe to close everything because we are assuming there are no
// requests in flight
@ -8305,12 +8304,12 @@ inline bool SSLClient::connect_with_proxy(Socket &socket, Response &res,
return false;
}
if (res2.status == 407) {
if (proxy_res.status == 407) {
if (!proxy_digest_auth_username_.empty() &&
!proxy_digest_auth_password_.empty()) {
std::map<std::string, std::string> auth;
if (detail::parse_www_authenticate(res2, auth, true)) {
Response res3;
if (detail::parse_www_authenticate(proxy_res, auth, true)) {
proxy_res = Response();
if (!detail::process_client_socket(
socket.sock, read_timeout_sec_, read_timeout_usec_,
write_timeout_sec_, write_timeout_usec_, [&](Stream &strm) {
@ -8321,7 +8320,7 @@ inline bool SSLClient::connect_with_proxy(Socket &socket, Response &res,
req3, auth, 1, detail::random_string(10),
proxy_digest_auth_username_, proxy_digest_auth_password_,
true));
return process_request(strm, req3, res3, false, error);
return process_request(strm, req3, proxy_res, false, error);
})) {
// Thread-safe to close everything because we are assuming there are
// no requests in flight
@ -8332,12 +8331,24 @@ inline bool SSLClient::connect_with_proxy(Socket &socket, Response &res,
return false;
}
}
} else {
res = res2;
return false;
}
}
// If status code is not 200, proxy request is failed.
// Set error to ProxyConnection and return proxy response
// as the response of the request
if (proxy_res.status != 200)
{
error = Error::ProxyConnection;
res = std::move(proxy_res);
// Thread-safe to close everything because we are assuming there are
// no requests in flight
shutdown_ssl(socket, true);
shutdown_socket(socket);
close_socket(socket);
return false;
}
return true;
}