Skip to content

Commit 095317c

Browse files
cjen1-msftcjen1-msft
authored
Merge branch 'main' into fix-cr-pem
2 parents 501fc04 + 3d7df00 commit 095317c

16 files changed

+217
-199
lines changed

‎.clang-tidy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,5 @@ Checks: >
5959
-readability-magic-numbers,
6060
6161
WarningsAsErrors: '*'
62-
HeaderFilterRegex: '(include\/ccf\/|src\/(udp|tcp|tls|tasks|snapshots|service|quic|pal|apps|clients|common|consensus|ds|enclave|endpoints|host|indexing)\/).*'
62+
HeaderFilterRegex: '(include\/ccf\/|src\/(udp|tcp|tls|tasks|snapshots|service|quic|pal|apps|clients|common|consensus|ds|enclave|endpoints|host|indexing|http)\/).*'
6363
FormatStyle: 'file'

‎src/http/curl.h‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ namespace ccf::curl
189189
unsent = std::span<const uint8_t>(buffer.data(), buffer.size());
190190
}
191191

192-
RequestBody(std::vector<uint8_t>&& buffer) : buffer(std::move(buffer))
192+
RequestBody(std::vector<uint8_t>&& buffer_) : buffer(std::move(buffer_))
193193
{
194194
unsent = std::span<const uint8_t>(buffer.data(), buffer.size());
195195
}
@@ -242,7 +242,7 @@ namespace ccf::curl
242242
ResponseBody(size_t max_size_) : maximum_size(max_size_) {}
243243

244244
static size_t write_response_chunk(
245-
uint8_t* ptr, size_t size, size_t nmemb, ResponseBody* response)
245+
const uint8_t* ptr, size_t size, size_t nmemb, ResponseBody* response)
246246
{
247247
if (response == nullptr)
248248
{
@@ -275,7 +275,7 @@ namespace ccf::curl
275275
}
276276

277277
static size_t noop_write_function(
278-
uint8_t* ptr, size_t size, size_t nmemb, ResponseBody* response)
278+
const uint8_t* ptr, size_t size, size_t nmemb, ResponseBody* response)
279279
{
280280
(void)ptr;
281281
(void)response;
@@ -392,7 +392,7 @@ namespace ccf::curl
392392
CurlRequest(
393393
UniqueCURL&& curl_handle_,
394394
RESTVerb method_,
395-
const std::string& url_,
395+
std::string url_,
396396
UniqueSlist&& headers_,
397397
std::unique_ptr<RequestBody>&& request_body_,
398398
std::unique_ptr<ccf::curl::ResponseBody>&& response_,
@@ -411,13 +411,14 @@ namespace ccf::curl
411411
}
412412
CHECK_CURL_EASY_SETOPT(curl_handle, CURLOPT_URL, url.c_str());
413413

414-
if (!method.get_http_method().has_value())
414+
auto http_method = method.get_http_method();
415+
if (!http_method.has_value())
415416
{
416417
throw std::logic_error(
417418
fmt::format("Unsupported HTTP method: {}", method.c_str()));
418419
}
419420

420-
switch (method.get_http_method().value())
421+
switch (http_method.value())
421422
{
422423
case HTTP_GET:
423424
CHECK_CURL_EASY_SETOPT(curl_handle, CURLOPT_HTTPGET, 1L);
@@ -473,13 +474,16 @@ namespace ccf::curl
473474
std::unique_ptr<CurlRequest>&& request, CURLcode curl_response_code)
474475
{
475476
LOG_TRACE_FMT("Handling response for {}", request->url);
476-
if (request->response_callback.has_value())
477+
auto& callback = request->response_callback;
478+
if (callback.has_value())
477479
{
478-
long status_code = 0;
479-
CHECK_CURL_EASY_GETINFO(
480-
request->curl_handle, CURLINFO_RESPONSE_CODE, &status_code);
481-
request->response_callback.value()(
482-
std::move(request), curl_response_code, status_code);
480+
if (callback.value() != nullptr)
481+
{
482+
long status_code = 0;
483+
CHECK_CURL_EASY_GETINFO(
484+
request->curl_handle, CURLINFO_RESPONSE_CODE, &status_code);
485+
callback.value()(std::move(request), curl_response_code, status_code);
486+
}
483487
}
484488
}
485489

@@ -955,7 +959,8 @@ namespace ccf::curl
955959
// remove, stop and cleanup all curl easy handles
956960
std::unique_ptr<CURL*, void (*)(CURL**)> easy_handles(
957961
curl_multi_get_handles(curl_request_curlm),
958-
[](CURL** handles) { curl_free(handles); });
962+
// NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
963+
[](CURL** handles) { curl_free(static_cast<void*>(handles)); });
959964
// curl_multi_get_handles returns the handles as a null-terminated array
960965
for (size_t i = 0; easy_handles.get()[i] != nullptr; ++i)
961966
{

‎src/http/error_reporter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace http
77
class ErrorReporter
88
{
99
public:
10-
virtual ~ErrorReporter() {}
10+
virtual ~ErrorReporter() = default;
1111
virtual void report_parsing_error(const ccf::ListenInterfaceID&) = 0;
1212
virtual void report_request_payload_too_large_error(
1313
const ccf::ListenInterfaceID&) = 0;

‎src/http/http2_callbacks.h‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ namespace http2
1616
uint8_t* buf,
1717
size_t length,
1818
uint32_t* data_flags,
19-
nghttp2_data_source* source,
20-
void* user_data)
19+
nghttp2_data_source* /*source*/,
20+
void* /*user_data*/)
2121
{
2222
auto* stream_data = get_stream_data(session, stream_id);
2323
if (stream_data->outgoing.state == StreamResponseState::Uninitialised)
@@ -64,7 +64,7 @@ namespace http2
6464
}
6565

6666
static int on_begin_frame_recv_callback(
67-
nghttp2_session* session, const nghttp2_frame_hd* hd, void* user_data)
67+
nghttp2_session* /*session*/, const nghttp2_frame_hd* hd, void* user_data)
6868
{
6969
const auto& stream_id = hd->stream_id;
7070
LOG_TRACE_FMT(
@@ -124,7 +124,7 @@ namespace http2
124124
}
125125

126126
// If the request is complete, process it
127-
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
127+
if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) != 0)
128128
{
129129
auto* p = get_parser(user_data);
130130
p->handle_completed(stream_id, stream_data);
@@ -176,7 +176,7 @@ namespace http2
176176
size_t namelen,
177177
const uint8_t* value,
178178
size_t valuelen,
179-
uint8_t flags,
179+
uint8_t /*flags*/,
180180
void* user_data)
181181
{
182182
const auto& stream_id = frame->hd.stream_id;
@@ -227,7 +227,7 @@ namespace http2
227227

228228
static int on_data_callback(
229229
nghttp2_session* session,
230-
uint8_t flags,
230+
uint8_t /*flags*/,
231231
StreamId stream_id,
232232
const uint8_t* data,
233233
size_t len,
@@ -257,7 +257,7 @@ namespace http2
257257
}
258258

259259
static int on_stream_close_callback(
260-
nghttp2_session* session,
260+
nghttp2_session* /*session*/,
261261
StreamId stream_id,
262262
uint32_t error_code,
263263
void* user_data)
@@ -272,8 +272,8 @@ namespace http2
272272
}
273273

274274
static ssize_t on_data_source_read_length_callback(
275-
nghttp2_session* session,
276-
uint8_t frame_type,
275+
nghttp2_session* /*session*/,
276+
uint8_t /*frame_type*/,
277277
int32_t stream_id,
278278
int32_t session_remote_window_size,
279279
int32_t stream_remote_window_size,
@@ -298,11 +298,11 @@ namespace http2
298298
}
299299

300300
static int on_error_callback(
301-
nghttp2_session* session,
302-
int lib_error_code,
301+
nghttp2_session* /*session*/,
302+
int /*lib_error_code*/,
303303
const char* msg,
304304
size_t len,
305-
void* user_data)
305+
void* /*user_data*/)
306306
{
307307
LOG_DEBUG_FMT("HTTP/2 error: {}", std::string(msg, msg + len));
308308
return 0;

‎src/http/http2_parser.h‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,16 @@ namespace http2
2525

2626
protected:
2727
std::map<StreamId, std::shared_ptr<StreamData>> streams;
28-
nghttp2_session* session;
28+
nghttp2_session* session = nullptr;
2929

3030
public:
3131
Parser(
32-
const ccf::http::ParserConfiguration& configuration_,
33-
bool is_client = false) :
34-
configuration(configuration_)
32+
ccf::http::ParserConfiguration configuration_, bool is_client = false) :
33+
configuration(std::move(configuration_))
3534
{
3635
LOG_TRACE_FMT("Creating HTTP2 parser");
3736

38-
nghttp2_session_callbacks* callbacks;
37+
nghttp2_session_callbacks* callbacks = nullptr;
3938
nghttp2_session_callbacks_new(&callbacks);
4039
nghttp2_session_callbacks_set_on_stream_close_callback(
4140
callbacks, on_stream_close_callback);
@@ -107,17 +106,18 @@ namespace http2
107106
nghttp2_session_callbacks_del(callbacks);
108107
}
109108

110-
virtual ~Parser()
109+
~Parser() override
111110
{
112111
nghttp2_session_del(session);
113112
}
114113

115-
StreamId get_last_stream_id() const override
114+
[[nodiscard]] StreamId get_last_stream_id() const override
116115
{
117116
return last_stream_id;
118117
}
119118

120-
ccf::http::ParserConfiguration get_configuration() const override
119+
[[nodiscard]] ccf::http::ParserConfiguration get_configuration()
120+
const override
121121
{
122122
return configuration;
123123
}
@@ -268,23 +268,22 @@ namespace http2
268268
void submit_response(
269269
StreamId stream_id,
270270
ccf::http_status status,
271-
const ccf::http::HeaderMap& base_headers,
271+
ccf::http::HeaderMap&& base_headers,
272272
const ccf::http::HeaderMap& extra_headers = {})
273273
{
274274
std::vector<nghttp2_nv> hdrs = {};
275275

276276
auto status_str = fmt::format(
277-
"{}",
278-
static_cast<std::underlying_type<ccf::http_status>::type>(status));
277+
"{}", static_cast<std::underlying_type_t<ccf::http_status>>(status));
279278
hdrs.emplace_back(
280279
make_nv(ccf::http2::headers::STATUS, status_str.data()));
281280

282-
for (auto& [k, v] : base_headers)
281+
for (const auto& [k, v] : base_headers)
283282
{
284283
hdrs.emplace_back(make_nv(k.data(), v.data()));
285284
}
286285

287-
for (auto& [k, v] : extra_headers)
286+
for (const auto& [k, v] : extra_headers)
288287
{
289288
hdrs.emplace_back(make_nv(k.data(), v.data()));
290289
}
@@ -327,7 +326,7 @@ namespace http2
327326
void respond(
328327
StreamId stream_id,
329328
ccf::http_status status,
330-
const ccf::http::HeaderMap& headers,
329+
ccf::http::HeaderMap&& headers,
331330
ccf::http::HeaderMap&& trailers,
332331
std::vector<uint8_t>&& body)
333332
{
@@ -365,7 +364,7 @@ namespace http2
365364

366365
if (should_submit_response)
367366
{
368-
submit_response(stream_id, status, headers, extra_headers);
367+
submit_response(stream_id, status, std::move(headers), extra_headers);
369368
send_all_submitted();
370369
}
371370

@@ -376,7 +375,7 @@ namespace http2
376375
void start_stream(
377376
StreamId stream_id,
378377
ccf::http_status status,
379-
const ccf::http::HeaderMap& headers)
378+
ccf::http::HeaderMap&& headers)
380379
{
381380
LOG_TRACE_FMT(
382381
"http2::start_stream: stream {} - {} headers",
@@ -398,7 +397,7 @@ namespace http2
398397

399398
stream_data->outgoing.state = StreamResponseState::Streaming;
400399

401-
submit_response(stream_id, status, headers);
400+
submit_response(stream_id, status, std::move(headers));
402401
send_all_submitted();
403402
}
404403

@@ -457,8 +456,7 @@ namespace http2
457456
// trailers
458457
}
459458

460-
virtual void handle_completed(
461-
StreamId stream_id, StreamData* stream_data) override
459+
void handle_completed(StreamId stream_id, StreamData* stream_data) override
462460
{
463461
LOG_TRACE_FMT("http2::ServerParser: handle_completed");
464462

@@ -484,7 +482,7 @@ namespace http2
484482
const auto method_it = headers.find(ccf::http2::headers::METHOD);
485483
if (method_it != headers.end())
486484
{
487-
method = ccf::http_method_from_str(method_it->second.c_str());
485+
method = ccf::http_method_from_str(method_it->second);
488486
}
489487
}
490488

@@ -548,7 +546,8 @@ namespace http2
548546
LOG_DEBUG_FMT("Successfully sent request with stream id: {}", stream_id);
549547
}
550548

551-
void handle_completed(StreamId stream_id, StreamData* stream_data) override
549+
void handle_completed(
550+
StreamId /*stream_id*/, StreamData* stream_data) override
552551
{
553552
LOG_TRACE_FMT("http2::ClientParser: handle_completed");
554553

0 commit comments

Comments
 (0)