Fix http2 stream pointer
Response and Request are now movable, so lets use that to our advantage
and make this no longer require a pointer. This removes a couple NOLINT
exceptions in our code, and cleans up a lot of places where we could
potentially get a nullptr.
Tested:
enabled http2-experimental option.
Loaded service root from redfish in curl with logging enabled, logging
verified http/2 was being used.
Redfish service validator passes.
Curl compiled with http returns service root correctly.
Change-Id: I65e11a2311be982df594086413d52838235e1a0c
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index 27df36e..4d1bcac 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -62,7 +62,7 @@
void start()
{
// Create the control stream
- streams.emplace(0, std::make_unique<Http2StreamData>());
+ streams[0];
if (sendServerConnectionHeader() != 0)
{
@@ -90,35 +90,35 @@
}
static ssize_t fileReadCallback(nghttp2_session* /* session */,
- int32_t /* stream_id */, uint8_t* buf,
+ int32_t streamId, uint8_t* buf,
size_t length, uint32_t* dataFlags,
- nghttp2_data_source* source,
- void* /*unused*/)
+ nghttp2_data_source* /*source*/,
+ void* userPtr)
{
- if (source == nullptr || source->ptr == nullptr)
+ self_type& self = userPtrToSelf(userPtr);
+
+ auto streamIt = self.streams.find(streamId);
+ if (streamIt == self.streams.end())
{
- BMCWEB_LOG_DEBUG("Source was null???");
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
-
+ Http2StreamData& stream = streamIt->second;
BMCWEB_LOG_DEBUG("File read callback length: {}", length);
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- Http2StreamData* str = reinterpret_cast<Http2StreamData*>(source->ptr);
- crow::Response& res = str->res;
+ crow::Response& res = stream.res;
BMCWEB_LOG_DEBUG("total: {} send_sofar: {}", res.body().size(),
- str->sentSofar);
+ stream.sentSofar);
- size_t toSend = std::min(res.body().size() - str->sentSofar, length);
+ size_t toSend = std::min(res.body().size() - stream.sentSofar, length);
BMCWEB_LOG_DEBUG("Copying {} bytes to buf", toSend);
std::string::iterator bodyBegin = res.body().begin();
- std::advance(bodyBegin, str->sentSofar);
+ std::advance(bodyBegin, stream.sentSofar);
memcpy(buf, &*bodyBegin, toSend);
- str->sentSofar += toSend;
+ stream.sentSofar += toSend;
- if (str->sentSofar >= res.body().size())
+ if (stream.sentSofar >= res.body().size())
{
BMCWEB_LOG_DEBUG("Setting OEF flag");
*dataFlags |= NGHTTP2_DATA_FLAG_EOF;
@@ -146,9 +146,9 @@
close();
return -1;
}
- Response& thisRes = it->second->res;
+ Response& thisRes = it->second.res;
thisRes = std::move(completedRes);
- crow::Request& thisReq = it->second->req;
+ crow::Request& thisReq = it->second.req;
std::vector<nghttp2_nv> hdr;
completeResponseFields(thisReq, thisRes);
@@ -162,13 +162,11 @@
hdr.emplace_back(
headerFromStringViews(header.name_string(), header.value()));
}
- Http2StreamData* streamPtr = it->second.get();
- streamPtr->sentSofar = 0;
+ Http2StreamData& stream = it->second;
+ stream.sentSofar = 0;
nghttp2_data_provider dataPrd{
- .source{
- .ptr = streamPtr,
- },
+ .source = {.fd = 0},
.read_callback = fileReadCallback,
};
@@ -210,11 +208,11 @@
return -1;
}
- crow::Request& thisReq = it->second->req;
+ crow::Request& thisReq = it->second.req;
BMCWEB_LOG_DEBUG("Handling {} \"{}\"", logPtr(&thisReq),
thisReq.url().encoded_path());
- crow::Response& thisRes = it->second->res;
+ crow::Response& thisRes = it->second.res;
thisRes.setCompleteRequestHandler(
[this, streamId](Response& completeRes) {
@@ -226,7 +224,7 @@
}
});
auto asyncResp =
- std::make_shared<bmcweb::AsyncResp>(std::move(it->second->res));
+ std::make_shared<bmcweb::AsyncResp>(std::move(it->second.res));
handler->handle(thisReq, asyncResp);
return 0;
@@ -287,13 +285,10 @@
BMCWEB_LOG_CRITICAL("user data was null?");
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
- auto stream = userPtrToSelf(userData).streams.find(streamId);
- if (stream == userPtrToSelf(userData).streams.end())
+ if (userPtrToSelf(userData).streams.erase(streamId) <= 0)
{
return -1;
}
-
- userPtrToSelf(userData).streams.erase(streamId);
return 0;
}
@@ -326,7 +321,7 @@
return -1;
}
- crow::Request& thisReq = thisStream->second->req;
+ crow::Request& thisReq = thisStream->second.req;
if (nameSv == ":path")
{
@@ -394,13 +389,9 @@
{
BMCWEB_LOG_DEBUG("create stream for id {}", frame.hd.stream_id);
- std::pair<boost::container::flat_map<
- int32_t, std::unique_ptr<Http2StreamData>>::iterator,
- bool>
- stream = streams.emplace(frame.hd.stream_id,
- std::make_unique<Http2StreamData>());
+ Http2StreamData& stream = streams[frame.hd.stream_id];
// http2 is by definition always tls
- stream.first->second->req.isSecure = true;
+ stream.req.isSecure = true;
}
return 0;
}
@@ -532,8 +523,7 @@
}
// A mapping from http2 stream ID to Stream Data
- boost::container::flat_map<int32_t, std::unique_ptr<Http2StreamData>>
- streams;
+ boost::container::flat_map<int32_t, Http2StreamData> streams;
boost::beast::multi_buffer sendBuffer;
boost::beast::multi_buffer inBuffer;