Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPENDENCIES
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
vendorpull https://github.com/sourcemeta/vendorpull 1dcbac42809cf87cb5b045106b863e17ad84ba02
uwebsockets https://github.com/uNetworking/uWebSockets v20.76.0
core https://github.com/sourcemeta/core bd4eb912ea0970c99a1caa2b3a75f44799ec1c38
core https://github.com/sourcemeta/core 53a18883063506c8b70cc2c137dd8a07b89ef647
blaze https://github.com/sourcemeta/blaze 83e2458856b934aa7995c44da36ab09152bdfc8d
jsonbinpack https://github.com/sourcemeta/jsonbinpack 2fe6502cad30e539de271d2e58ee7f8432b95a4d
hydra https://github.com/sourcemeta/hydra 8cbbe5739c31d282a822dece8ed36ef0120cccbe
Expand Down
7 changes: 1 addition & 6 deletions src/actions/action_default_v1.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ class ActionDefault_v1 : public sourcemeta::one::Action {
const sourcemeta::core::URITemplateRouterView &router,
const sourcemeta::core::URITemplateRouter::Identifier identifier)
: sourcemeta::one::Action{base, router.base_path()} {
// TODO: This implies the API is mounted
this->error_schema_ =
std::string{this->base_path()} + "/self/v1/schemas/api/error";

router.arguments(identifier, [this](const auto &key, const auto &value) {
if (key == "errorSchema") {
this->error_schema_ = std::get<std::string_view>(value);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error_schema_ is a default-initialized std::string_view, so if the router has no errorSchema argument for this identifier it stays empty and later json_error/serve calls will emit responses without a schema URI. Other locations where this applies: src/actions/dispatch.cc:53, src/actions/dispatch.cc:73.

Severity: medium

Other Locations
  • src/actions/dispatch.cc:53
  • src/actions/dispatch.cc:73

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Expand Down Expand Up @@ -115,8 +111,7 @@ class ActionDefault_v1 : public sourcemeta::one::Action {
}

private:
// TODO: This should be a string view
std::string error_schema_;
std::string_view error_schema_;
};

#endif
67 changes: 34 additions & 33 deletions src/actions/dispatch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
#include "action_serve_schema_artifact_v1.h"
#include "action_serve_static_v1.h"

#include <array> // std::array
#include <cstddef> // std::size_t
#include <memory> // std::unique_ptr, std::make_unique
#include <mutex> // std::once_flag, std::call_once
#include <string> // std::string
#include <array> // std::array
#include <memory> // std::unique_ptr, std::make_unique
#include <mutex> // std::once_flag, std::call_once
#include <string> // std::string

template <typename T>
static auto
Expand Down Expand Up @@ -44,45 +43,47 @@ static constexpr std::array<ActionConstructFunction,

#undef SOURCEMETA_ONE_MAKE_CONSTRUCTOR_ENTRY

struct Slot {
std::unique_ptr<sourcemeta::one::Action> instance;
std::once_flag flag;
};

// Heap array because Slot contains a non-movable std::once_flag
// NOLINTNEXTLINE(modernize-avoid-c-arrays)
static std::unique_ptr<Slot[]> SLOTS;
static std::size_t SLOTS_SIZE{0};
sourcemeta::one::ActionDispatcher::ActionDispatcher(
const std::filesystem::path &base,
const sourcemeta::core::URITemplateRouterView &router)
: base_{base}, router_{router},
// NOLINTNEXTLINE(modernize-avoid-c-arrays)
slots_{std::make_unique<Slot[]>(router.size() + 1)},
slots_size_{router.size() + 1} {
router.arguments(0, [this](const auto &key, const auto &value) {
if (key == "errorSchema") {
this->default_error_schema_ = std::get<std::string_view>(value);
}
});
}

auto sourcemeta::one::actions_initialize(
const sourcemeta::core::URITemplateRouterView &router) -> void {
SLOTS_SIZE = router.size() + 1;
// NOLINTNEXTLINE(modernize-avoid-c-arrays)
SLOTS = std::make_unique<Slot[]>(SLOTS_SIZE);
auto sourcemeta::one::ActionDispatcher::error(
const sourcemeta::one::HTTPRequest &request,
sourcemeta::one::HTTPResponse &response, const char *const code,
std::string &&identifier, std::string &&message) const -> void {
sourcemeta::one::json_error(request, response, code, std::move(identifier),
std::move(message), this->default_error_schema_);
}

auto sourcemeta::one::actions_dispatch(
auto sourcemeta::one::ActionDispatcher::dispatch(
const sourcemeta::core::URITemplateRouter::Identifier identifier,
const sourcemeta::core::URITemplateRouter::Identifier context,
const sourcemeta::core::URITemplateRouterView &router,
const std::filesystem::path &base,
const std::span<std::string_view> matches,
sourcemeta::one::HTTPRequest &request,
sourcemeta::one::HTTPResponse &response) -> void {
if (identifier >= SLOTS_SIZE || context >= CONSTRUCTORS.size()) [[unlikely]] {
sourcemeta::one::json_error(
request, response, sourcemeta::one::STATUS_NOT_IMPLEMENTED,
"unknown-handler-code",
"This server version does not implement the handler for "
"this URL",
// TODO: This implies the API is mounted
std::string{router.base_path()} + "/self/v1/schemas/api/error");
if (identifier >= this->slots_size_ || context >= CONSTRUCTORS.size())
[[unlikely]] {
this->error(request, response, sourcemeta::one::STATUS_NOT_IMPLEMENTED,
"unknown-handler-code",
"This server version does not implement the handler for "
"this URL");
return;
}

auto &slot{SLOTS[identifier]};
std::call_once(slot.flag, [&] {
slot.instance = CONSTRUCTORS[context](base, router, identifier);
auto &slot{this->slots_[identifier]};
std::call_once(slot.flag, [this, &slot, context, identifier] {
slot.instance =
CONSTRUCTORS[context](this->base_, this->router_, identifier);
});

slot.instance->run(matches, request, response);
Expand Down
45 changes: 38 additions & 7 deletions src/actions/include/sourcemeta/one/actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@

#include <sourcemeta/one/http.h>

#include <cstddef> // std::size_t
#include <cstdint> // std::uint8_t
#include <filesystem> // std::filesystem::path
#include <memory> // std::unique_ptr
#include <mutex> // std::once_flag
#include <optional> // std::optional
#include <span> // std::span
#include <string_view> // std::string_view
Expand Down Expand Up @@ -64,14 +67,42 @@ class Action {
std::string_view base_path_;
};

auto actions_initialize(const core::URITemplateRouterView &router) -> void;
class ActionDispatcher {
public:
ActionDispatcher(const std::filesystem::path &base,
const core::URITemplateRouterView &router);
~ActionDispatcher() = default;

// To avoid mistakes
ActionDispatcher(const ActionDispatcher &) = delete;
ActionDispatcher(ActionDispatcher &&) = delete;
auto operator=(const ActionDispatcher &) -> ActionDispatcher & = delete;
auto operator=(ActionDispatcher &&) -> ActionDispatcher & = delete;

auto actions_dispatch(const core::URITemplateRouter::Identifier identifier,
const core::URITemplateRouter::Identifier context,
const core::URITemplateRouterView &router,
const std::filesystem::path &base,
const std::span<std::string_view> matches,
HTTPRequest &request, HTTPResponse &response) -> void;
auto dispatch(const core::URITemplateRouter::Identifier identifier,
const core::URITemplateRouter::Identifier context,
const std::span<std::string_view> matches, HTTPRequest &request,
HTTPResponse &response) -> void;

auto error(const HTTPRequest &request, HTTPResponse &response,
const char *const code, std::string &&identifier,
std::string &&message) const -> void;

private:
struct Slot {
std::unique_ptr<Action> instance;
std::once_flag flag;
};

// NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
const std::filesystem::path &base_;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActionDispatcher stores base_/router_ by reference, so constructing it with temporaries (or with objects that don’t outlive the server) would leave dangling references and can become a UAF later. Consider documenting/enforcing the required lifetime at the API boundary.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

// NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
const core::URITemplateRouterView &router_;
// NOLINTNEXTLINE(modernize-avoid-c-arrays)
std::unique_ptr<Slot[]> slots_;
std::size_t slots_size_;
std::string_view default_error_schema_;
};

} // namespace sourcemeta::one

Expand Down
5 changes: 5 additions & 0 deletions src/index/generators.h
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,11 @@ struct GENERATE_URITEMPLATE_ROUTES {
const auto error_schema{configuration.base_path +
"/self/v1/schemas/api/error"};

const sourcemeta::core::URITemplateRouter::Argument otherwise_arguments[] =
{{"errorSchema", std::string_view{error_schema}}};
router.otherwise(sourcemeta::one::ACTION_TYPE_DEFAULT_V1,
otherwise_arguments);

sourcemeta::core::URITemplateRouter::Identifier next_id{1};

const sourcemeta::core::URITemplateRouter::Argument list_arguments[] = {
Expand Down
44 changes: 19 additions & 25 deletions src/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
#include <string> // std::string, std::to_string
#include <string_view> // std::string_view

static auto dispatch(const sourcemeta::core::URITemplateRouterView &router,
const std::filesystem::path &base,
// TODO: Maybe we should merge this entire function into `ActionDispatcher`?
static auto dispatch(sourcemeta::one::ActionDispatcher &actions,
const sourcemeta::core::URITemplateRouterView &router,
sourcemeta::one::HTTPRequest &request,
sourcemeta::one::HTTPResponse &response) noexcept -> void {
try {
Expand All @@ -34,29 +35,22 @@ static auto dispatch(const sourcemeta::core::URITemplateRouterView &router,
matches_size = static_cast<std::size_t>(index) + 1;
})};

sourcemeta::one::actions_dispatch(
match_result.first, match_result.second, router, base,
std::span{matches.data(), matches_size}, request, response);
actions.dispatch(match_result.first, match_result.second,
std::span{matches.data(), matches_size}, request,
response);
} else {
sourcemeta::one::json_error(
request, response, sourcemeta::one::STATUS_NOT_ACCEPTABLE,
"cannot-satisfy-content-encoding",
"The server cannot satisfy the request content encoding",
// TODO: This implies the API is mounted
"/self/v1/schemas/api/error");
actions.error(request, response, sourcemeta::one::STATUS_NOT_ACCEPTABLE,
"cannot-satisfy-content-encoding",
"The server cannot satisfy the request content encoding");
}
} catch (const std::exception &error) {
sourcemeta::one::json_error(request, response,
sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR,
"uncaught-error", error.what(),
// TODO: This implies the API is mounted
"/self/v1/schemas/api/error");
actions.error(request, response,
sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR,
"uncaught-error", error.what());
} catch (...) {
sourcemeta::one::json_error(
request, response, sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR,
"uncaught-error", "An unknown unexpected error occurred",
// TODO: This implies the API is mounted
"/self/v1/schemas/api/error");
actions.error(request, response,
sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR,
"uncaught-error", "An unknown unexpected error occurred");
}
}

Expand Down Expand Up @@ -100,13 +94,13 @@ auto main(int argc, char *argv[]) noexcept -> int {
}

const sourcemeta::core::URITemplateRouterView router{base / "routes.bin"};
sourcemeta::one::actions_initialize(router);
sourcemeta::one::ActionDispatcher actions{base, router};

sourcemeta::one::HTTPServer(
port,
[&router, &base](sourcemeta::one::HTTPRequest &request,
sourcemeta::one::HTTPResponse &response) {
dispatch(router, base, request, response);
[&actions, &router](sourcemeta::one::HTTPRequest &request,
sourcemeta::one::HTTPResponse &response) {
dispatch(actions, router, request, response);
},
[timestamp_start](const std::uint16_t bound_port) {
const auto duration{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading