diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | src/zenhttp/httpserver.cpp | 87 | ||||
| -rw-r--r-- | src/zenserver-test/objectstore-tests.cpp | 74 |
3 files changed, 148 insertions, 14 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 899415d3a..777696097 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ default will be WARN no matter what you pass on the command line. - Bugfix: `--plain-progress` style progress bar should now show elapsed time correctly - Bugfix: Time spent indexing local and remote state during `zen builds download` now show the correct time +- Bugfix: ObjectStore failed to correctly parse urls with sub folder paths causing 404 to be returned ## 5.7.21 - Feature: Added `--security-config-path` option to zenserver to configure security settings diff --git a/src/zenhttp/httpserver.cpp b/src/zenhttp/httpserver.cpp index 2facd8401..d798c46d9 100644 --- a/src/zenhttp/httpserver.cpp +++ b/src/zenhttp/httpserver.cpp @@ -746,6 +746,10 @@ HttpRequestRouter::RegisterRoute(const char* UriPattern, HttpRequestRouter::Hand { if (UriPattern[i] == '}') { + if (i == PatternStart) + { + throw std::runtime_error(fmt::format("matcher pattern is empty in URI pattern '{}'", UriPattern)); + } std::string_view Pattern(&UriPattern[PatternStart], i - PatternStart); if (auto it = m_MatcherNameMap.find(std::string(Pattern)); it != m_MatcherNameMap.end()) { @@ -911,8 +915,9 @@ HttpRequestRouter::HandleRequest(zen::HttpServerRequest& Request) CapturedSegments.emplace_back(Uri); - for (int MatcherIndex : Matchers) + for (size_t MatcherOffset = 0; MatcherOffset < Matchers.size(); MatcherOffset++) { + int MatcherIndex = Matchers[MatcherOffset]; if (UriPos >= UriLen) { IsMatch = false; @@ -922,9 +927,9 @@ HttpRequestRouter::HandleRequest(zen::HttpServerRequest& Request) if (MatcherIndex < 0) { // Literal match - int LitIndex = -MatcherIndex - 1; - const std::string& LitStr = m_Literals[LitIndex]; - size_t LitLen = LitStr.length(); + int LitIndex = -MatcherIndex - 1; + std::string_view LitStr = m_Literals[LitIndex]; + size_t LitLen = LitStr.length(); if (Uri.substr(UriPos, LitLen) == LitStr) { @@ -940,9 +945,18 @@ HttpRequestRouter::HandleRequest(zen::HttpServerRequest& Request) { // Matcher function size_t SegmentStart = UriPos; - while (UriPos < UriLen && Uri[UriPos] != '/') + + if (MatcherOffset == (Matchers.size() - 1)) + { + // Last matcher, use the remaining part of the uri + UriPos = UriLen; + } + else { - ++UriPos; + while (UriPos < UriLen && Uri[UriPos] != '/') + { + ++UriPos; + } } std::string_view Segment = Uri.substr(SegmentStart, UriPos - SegmentStart); @@ -1429,20 +1443,33 @@ TEST_CASE("http.common") SUBCASE("router-matcher") { - bool HandledA = false; - bool HandledAA = false; - bool HandledAB = false; - bool HandledAandB = false; + bool HandledA = false; + bool HandledAA = false; + bool HandledAB = false; + bool HandledAandB = false; + bool HandledAandPath = false; std::vector<std::string> Captures; auto Reset = [&] { - HandledA = HandledAA = HandledAB = HandledAandB = false; + HandledA = HandledAA = HandledAB = HandledAandB = HandledAandPath = false; Captures.clear(); }; TestHttpService Service; HttpRequestRouter r; - r.AddMatcher("a", [](std::string_view In) -> bool { return In.length() % 2 == 0; }); - r.AddMatcher("b", [](std::string_view In) -> bool { return In.length() % 3 == 0; }); + + r.AddMatcher("a", [](std::string_view In) -> bool { return In.length() % 2 == 0 && In.find('/') == std::string_view::npos; }); + r.AddMatcher("b", [](std::string_view In) -> bool { return In.length() % 3 == 0 && In.find('/') == std::string_view::npos; }); + static constexpr AsciiSet ValidPathCharactersSet{"abcdefghijklmnopqrstuvwxyz0123456789/_.,;$~{}+-[]%()]ABCDEFGHIJKLMNOPQRSTUVWXYZ"}; + r.AddMatcher("path", [](std::string_view Str) -> bool { return !Str.empty() && AsciiSet::HasOnly(Str, ValidPathCharactersSet); }); + + r.RegisterRoute( + "path/{a}/{path}", + [&](auto& Req) { + HandledAandPath = true; + Captures = {std::string(Req.GetCapture(1)), std::string(Req.GetCapture(2))}; + }, + HttpVerb::kGet); + r.RegisterRoute( "{a}", [&](auto& Req) { @@ -1471,7 +1498,6 @@ TEST_CASE("http.common") Captures = {std::string(Req.GetCapture(1)), std::string(Req.GetCapture(2))}; }, HttpVerb::kGet); - { Reset(); TestHttpServerRequest req{Service, "ab"sv}; @@ -1479,6 +1505,7 @@ TEST_CASE("http.common") CHECK(HandledA); CHECK(!HandledAA); CHECK(!HandledAB); + CHECK(!HandledAandPath); REQUIRE_EQ(Captures.size(), 1); CHECK_EQ(Captures[0], "ab"sv); @@ -1491,6 +1518,7 @@ TEST_CASE("http.common") CHECK(!HandledA); CHECK(!HandledAA); CHECK(HandledAB); + CHECK(!HandledAandPath); REQUIRE_EQ(Captures.size(), 2); CHECK_EQ(Captures[0], "ab"sv); CHECK_EQ(Captures[1], "def"sv); @@ -1504,6 +1532,7 @@ TEST_CASE("http.common") CHECK(!HandledAA); CHECK(!HandledAB); CHECK(HandledAandB); + CHECK(!HandledAandPath); REQUIRE_EQ(Captures.size(), 2); CHECK_EQ(Captures[0], "ab"sv); CHECK_EQ(Captures[1], "def"sv); @@ -1516,6 +1545,7 @@ TEST_CASE("http.common") CHECK(!HandledA); CHECK(!HandledAA); CHECK(!HandledAB); + CHECK(!HandledAandPath); } { @@ -1525,6 +1555,35 @@ TEST_CASE("http.common") CHECK(HandledA); CHECK(!HandledAA); CHECK(!HandledAB); + CHECK(!HandledAandPath); + REQUIRE_EQ(Captures.size(), 1); + CHECK_EQ(Captures[0], "a123"sv); + } + + { + Reset(); + TestHttpServerRequest req{Service, "path/ab/simple_path.txt"sv}; + r.HandleRequest(req); + CHECK(!HandledA); + CHECK(!HandledAA); + CHECK(!HandledAB); + CHECK(HandledAandPath); + REQUIRE_EQ(Captures.size(), 2); + CHECK_EQ(Captures[0], "ab"sv); + CHECK_EQ(Captures[1], "simple_path.txt"sv); + } + + { + Reset(); + TestHttpServerRequest req{Service, "path/ab/directory/and/path.txt"sv}; + r.HandleRequest(req); + CHECK(!HandledA); + CHECK(!HandledAA); + CHECK(!HandledAB); + CHECK(HandledAandPath); + REQUIRE_EQ(Captures.size(), 2); + CHECK_EQ(Captures[0], "ab"sv); + CHECK_EQ(Captures[1], "directory/and/path.txt"sv); } } diff --git a/src/zenserver-test/objectstore-tests.cpp b/src/zenserver-test/objectstore-tests.cpp new file mode 100644 index 000000000..f3db5fdf6 --- /dev/null +++ b/src/zenserver-test/objectstore-tests.cpp @@ -0,0 +1,74 @@ +// Copyright Epic Games, Inc. All Rights Reserved. + +#if ZEN_WITH_TESTS +# include "zenserver-test.h" +# include <zencore/testing.h> +# include <zencore/testutils.h> +# include <zenutil/zenserverprocess.h> +# include <zenhttp/httpclient.h> + +ZEN_THIRD_PARTY_INCLUDES_START +# include <tsl/robin_set.h> +ZEN_THIRD_PARTY_INCLUDES_END + +namespace zen::tests { + +using namespace std::literals; + +TEST_SUITE_BEGIN("server.objectstore"); + +TEST_CASE("objectstore.blobs") +{ + std::string_view Bucket = "bkt"sv; + + std::vector<IoHash> CompressedBlobsHashes; + std::vector<uint64_t> BlobsSizes; + std::vector<uint64_t> CompressedBlobsSizes; + { + ZenServerInstance Instance(TestEnv); + + const uint16_t PortNumber = Instance.SpawnServerAndWaitUntilReady(fmt::format("--objectstore-enabled")); + CHECK(PortNumber != 0); + + HttpClient Client(Instance.GetBaseUri() + "/obj/"); + + for (size_t I = 0; I < 5; I++) + { + IoBuffer Blob = CreateSemiRandomBlob(4711 + I * 7); + BlobsSizes.push_back(Blob.GetSize()); + CompressedBuffer CompressedBlob = CompressedBuffer::Compress(SharedBuffer(std::move(Blob))); + CompressedBlobsHashes.push_back(CompressedBlob.DecodeRawHash()); + CompressedBlobsSizes.push_back(CompressedBlob.GetCompressedSize()); + IoBuffer Payload = std::move(CompressedBlob).GetCompressed().Flatten().AsIoBuffer(); + Payload.SetContentType(ZenContentType::kCompressedBinary); + + std::string ObjectPath = fmt::format("{}/{}.utoc", + CompressedBlobsHashes.back().ToHexString().substr(0, 2), + CompressedBlobsHashes.back().ToHexString()); + + HttpClient::Response Result = Client.Put(fmt::format("bucket/{}/{}.utoc", Bucket, ObjectPath), Payload); + CHECK(Result); + } + + for (size_t I = 0; I < 5; I++) + { + std::string ObjectPath = + fmt::format("{}/{}.utoc", CompressedBlobsHashes[I].ToHexString().substr(0, 2), CompressedBlobsHashes[I].ToHexString()); + HttpClient::Response Result = Client.Get(fmt::format("bucket/{}/{}.utoc", Bucket, ObjectPath)); + CHECK(Result); + CHECK_EQ(Result.ResponsePayload.GetSize(), CompressedBlobsSizes[I]); + IoHash RawHash; + uint64_t RawSize; + CompressedBuffer Compressed = + CompressedBuffer::FromCompressed(SharedBuffer(std::move(Result.ResponsePayload)), RawHash, RawSize); + CHECK(Compressed); + CHECK_EQ(RawHash, CompressedBlobsHashes[I]); + CHECK_EQ(RawSize, BlobsSizes[I]); + } + } +} + +TEST_SUITE_END(); + +} // namespace zen::tests +#endif |