From efead0ae73a95ad0d54b53946d524d57309eb737 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Wed, 15 Jan 2025 11:44:55 -0500 Subject: [PATCH 01/28] Parts of implementation using ada-url instead of uriparser --- CMakeLists.txt | 5 +- CesiumUtility/CMakeLists.txt | 2 +- CesiumUtility/src/Uri.cpp | 420 +++++++++-------------------------- 3 files changed, 111 insertions(+), 316 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 17e780cf5..23abf4903 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,8 +44,8 @@ message(STATUS "VCPKG_OVERLAY_TRIPLETS ${VCPKG_OVERLAY_TRIPLETS}") # These packages are used in the public headers of Cesium libraries, so we need to distribute the headers and binaries # with the installation # Note that fmt is a public dependency of the vcpkg version of spdlog -# STB and uriparser aren't technically part of the public interface, but they're used by the downstream Cesium for Unreal project -set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb uriparser) +# STB and ada-url aren't technically part of the public interface, but they're used by the downstream Cesium for Unreal project +set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url) # These packages are used in the code and produce binaries, but are not part of the public interface. Therefore we need # to distribute the binaries for linking, but not the headers, as downstream consumers don't need them # OpenSSL and abseil are both dependencies of s2geometry @@ -263,6 +263,7 @@ find_package(tinyxml2 CONFIG REQUIRED) find_package(unofficial-sqlite3 CONFIG REQUIRED) find_package(uriparser CONFIG REQUIRED char wchar_t) find_package(WebP CONFIG REQUIRED) +find_package(ada CONFIG REQUIRED) # Private Library (s2geometry) diff --git a/CesiumUtility/CMakeLists.txt b/CesiumUtility/CMakeLists.txt index edbc3a0be..ed270219a 100644 --- a/CesiumUtility/CMakeLists.txt +++ b/CesiumUtility/CMakeLists.txt @@ -46,5 +46,5 @@ target_link_libraries( zlib-ng::zlib-ng spdlog::spdlog glm::glm - uriparser::uriparser + ada::ada ) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index f311ab759..7075d2b24 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -1,110 +1,105 @@ #include #include -#include -#include +#include +#include +#include +#include +#include #include #include #include +#include #include +#include +#include #include #include #include namespace CesiumUtility { +using UrlResult = ada::result; + namespace { -const char* const HTTPS_PREFIX = "https:"; -const char* const HTTP_PREFIX = "http:"; +void conformWindowsPathInPlace(std::string& path) { + // Treated as a Unix path, c:/file.txt will be /c:/file.txt. + if (path.length() >= 3 && path[0] == '/' && std::isalpha(path[1]) && + path[2] == ':') { + path.erase(0, 1); + } -std::string cesiumConformUrl(const std::string& url, bool useHttps) { - // Prepend protocol to protocol-relative URIs. - if (url.length() > 2 && url.at(0) == '/' && url.at(1) == '/') { - return std::string(useHttps ? HTTPS_PREFIX : HTTP_PREFIX).append(url); + // Replace slashes in-place + for (size_t i = 0; i < path.length(); i++) { + if (path[i] == '/') { + path[i] = '\\'; + } } - return url; } -} // namespace -std::string Uri::resolve( - const std::string& base, - const std::string& relative, - bool useBaseQuery, - bool assumeHttpsDefault) { - const std::string conformedBase = cesiumConformUrl(base, assumeHttpsDefault); - const std::string conformedRelative = - cesiumConformUrl(relative, assumeHttpsDefault); - UriUriA baseUri; - - if (uriParseSingleUriA(&baseUri, conformedBase.c_str(), nullptr) != - URI_SUCCESS) { - // Could not parse the base, so just use the relative directly and hope for - // the best. - return relative; +std::string urlEncode(const std::string_view& value) { + std::ostringstream output; + output << std::hex; + output.fill('0'); + + for (size_t i = 0; i < value.length(); i++) { + // RFC 3986 unreserved characters + if (std::isalnum(value[i]) || value[i] == '-' || value[i] == '.' || + value[i] == '_' || value[i] == '~') { + output << value[i]; + } else { + output << std::uppercase; + output << '%'; + output << std::setw(2) << value[i]; + output << std::nouppercase; + } } - UriUriA relativeUri; - if (uriParseSingleUriA(&relativeUri, conformedRelative.c_str(), nullptr) != - URI_SUCCESS) { - // Could not parse one of the URLs, so just use the relative directly and - // hope for the best. - uriFreeUriMembersA(&baseUri); - return relative; - } + return output.str(); +} - UriUriA resolvedUri; - if (uriAddBaseUriA(&resolvedUri, &relativeUri, &baseUri) != URI_SUCCESS) { - uriFreeUriMembersA(&resolvedUri); - uriFreeUriMembersA(&relativeUri); - uriFreeUriMembersA(&baseUri); - return relative; +const char* const HTTPS_PREFIX = "https:"; +const char* const HTTP_PREFIX = "http:"; + +UrlResult parseUrlConform(const std::string& url, bool assumeHttpsDefault) { + if (url.starts_with("//")) { + return ada::parse( + std::string(assumeHttpsDefault ? HTTPS_PREFIX : HTTP_PREFIX) + url); } + return ada::parse(url); +} - if (uriNormalizeSyntaxA(&resolvedUri) != URI_SUCCESS) { - uriFreeUriMembersA(&resolvedUri); - uriFreeUriMembersA(&relativeUri); - uriFreeUriMembersA(&baseUri); - return relative; +UrlResult parseUrlConform(const std::string& url, const ada::url_aggregator* base, bool assumeHttpsDefault) { + if (url.starts_with("//")) { + return ada::parse( + std::string(assumeHttpsDefault ? HTTPS_PREFIX : HTTP_PREFIX) + url, base); } + return ada::parse(url, base); +} +} // namespace - int charsRequired; - if (uriToStringCharsRequiredA(&resolvedUri, &charsRequired) != URI_SUCCESS) { - uriFreeUriMembersA(&resolvedUri); - uriFreeUriMembersA(&relativeUri); - uriFreeUriMembersA(&baseUri); +std::string Uri::resolve( + const std::string& base, + const std::string& relative, + bool useBaseQuery, + [[maybe_unused]] bool assumeHttpsDefault) { + UrlResult baseUrl = parseUrlConform(base, assumeHttpsDefault); + if (!baseUrl) { return relative; } - std::string result(static_cast(charsRequired), ' '); - - if (uriToStringA( - const_cast(result.c_str()), - &resolvedUri, - charsRequired + 1, - nullptr) != URI_SUCCESS) { - uriFreeUriMembersA(&resolvedUri); - uriFreeUriMembersA(&relativeUri); - uriFreeUriMembersA(&baseUri); + UrlResult relativeUrl = + parseUrlConform(relative, &baseUrl.value(), assumeHttpsDefault); + if (!relativeUrl) { return relative; } if (useBaseQuery) { - std::string query(baseUri.query.first, baseUri.query.afterLast); - if (query.length() > 0) { - if (resolvedUri.query.first) { - result += "&" + query; - } else { - result += "?" + query; - } - } + relativeUrl->set_search(baseUrl->get_search()); } - uriFreeUriMembersA(&resolvedUri); - uriFreeUriMembersA(&relativeUri); - uriFreeUriMembersA(&baseUri); - - return result; + return std::string(relativeUrl->get_href()); } std::string Uri::addQuery( @@ -116,50 +111,16 @@ std::string Uri::addQuery( return uri + "&" + key + "=" + value; } return uri + "?" + key + "=" + value; - // UriUriA baseUri; - - // if (uriParseSingleUriA(&baseUri, uri.c_str(), nullptr) != URI_SUCCESS) - //{ - // // TODO: report error - // return uri; - //} - - // uriFreeUriMembersA(&baseUri); } std::string Uri::getQueryValue(const std::string& url, const std::string& key) { - // We need to conform the URL since it will fail parsing if it's - // protocol-relative. However, it doesn't matter what protocol we use since - // it's only extracting query parameters. - const std::string conformedUrl = cesiumConformUrl(url, true); - UriUriA uri; - if (uriParseSingleUriA(&uri, conformedUrl.c_str(), nullptr) != URI_SUCCESS) { + UrlResult parsedUrl = parseUrlConform(url, true); + if (!parsedUrl) { return ""; } - UriQueryListA* queryList; - int itemCount; - if (uriDissectQueryMallocA( - &queryList, - &itemCount, - uri.query.first, - uri.query.afterLast) != URI_SUCCESS) { - uriFreeUriMembersA(&uri); - return ""; - } - UriQueryListA* p = queryList; - while (p) { - if (p->key && std::strcmp(p->key, key.c_str()) == 0) { - std::string value = p->value ? p->value : ""; - uriUnescapeInPlaceA(value.data()); - uriFreeQueryListA(queryList); - uriFreeUriMembersA(&uri); - return value; - } - p = p->next; - } - uriFreeQueryListA(queryList); - uriFreeUriMembersA(&uri); - return ""; + + ada::url_search_params params(parsedUrl->get_search()); + return std::string(params.get(key).value_or("")); } std::string Uri::substituteTemplateParameters( @@ -193,241 +154,74 @@ std::string Uri::substituteTemplateParameters( return result; } -std::string Uri::escape(const std::string& s) { - // In the worst case, escaping causes each character to turn into three. - std::string result(s.size() * 3, '\0'); - char* pTerminator = uriEscapeExA( - s.data(), - s.data() + s.size(), - result.data(), - URI_FALSE, - URI_FALSE); - result.resize(size_t(pTerminator - result.data())); - return result; -} +std::string Uri::escape(const std::string& s) { return urlEncode(s); } std::string Uri::unescape(const std::string& s) { - std::string result = s; - const char* pNewNull = - uriUnescapeInPlaceExA(result.data(), URI_FALSE, URI_BR_DONT_TOUCH); - result.resize(size_t(pNewNull - result.data())); - return result; + return ada::unicode::percent_decode(s, s.find('%')); } std::string Uri::unixPathToUriPath(const std::string& unixPath) { - // UriParser docs: - // The destination buffer must be large enough to hold 7 + 3 * len(filename) - // + 1 characters in case of an absolute filename or 3 * len(filename) + 1 - // in case of a relative filename. - std::string result(7 + 3 * unixPath.size() + 1, '\0'); - if (uriUnixFilenameToUriStringA(unixPath.data(), result.data()) != 0) { - // Error - return original string. - return unixPath; - } else { - // An absolute URI will start with "file://". Remove this. - if (result.find("file://", 0, 7) != std::string::npos) { - result.erase(0, 7); - } - - // Truncate at first null character - result.resize(std::strlen(result.data())); - return result; - } + return nativePathToUriPath(unixPath); } std::string Uri::windowsPathToUriPath(const std::string& windowsPath) { - // uriWindowsFilenameToUriStringA doesn't allow `/` character in the path (it - // percent encodes them) even though that's a perfectly valid path separator - // on Windows. So convert all forward slashes to back slashes before calling - // it. - std::string windowsPathClean; - windowsPathClean.resize(windowsPath.size()); - std::replace_copy( - windowsPath.begin(), - windowsPath.end(), - windowsPathClean.begin(), - '/', - '\\'); - - // UriParser docs: - // The destination buffer must be large enough to hold 8 + 3 * len(filename) - // + 1 characters in case of an absolute filename or 3 * len(filename) + 1 - // in case of a relative filename. - std::string result(8 + 3 * windowsPathClean.size() + 1, '\0'); - - if (uriWindowsFilenameToUriStringA(windowsPathClean.data(), result.data()) != - 0) { - // Error - return original string. - return windowsPath; - } else { - // An absolute URI will start with "file://". Remove this. - if (result.find("file://", 0, 7) != std::string::npos) { - result.erase(0, 7); - } - - // Truncate at first null character - result.resize(std::strlen(result.data())); - return result; - } + return nativePathToUriPath(windowsPath); } std::string Uri::nativePathToUriPath(const std::string& nativePath) { -#ifdef _WIN32 - return windowsPathToUriPath(nativePath); -#else - return unixPathToUriPath(nativePath); -#endif + UrlResult parsedUrl = ada::parse("file://" + nativePath); + if (!parsedUrl) { + return nativePath; + } + + std::string result(parsedUrl->get_pathname()); + return result; } std::string Uri::uriPathToUnixPath(const std::string& uriPath) { - // UriParser docs: - // The destination buffer must be large enough to hold len(uriString) + 1 - // - 5 characters in case of an absolute URI or len(uriString) + 1 in case - // of a relative URI. - // However, the above seems to assume that uriPath starts with "file:", which - // is not required. - std::string result(uriPath.size() + 1, '\0'); - if (uriUriStringToUnixFilenameA(uriPath.data(), result.data()) != 0) { - // Error - return original string. - return uriPath; - } else { - // Truncate at first null character - result.resize(std::strlen(result.data())); - return result; + ada::url_aggregator url; + url.set_pathname(uriPath); + std::string result = ada::unicode::percent_decode( + url.get_pathname(), + url.get_pathname().find('%')); + if (result.starts_with("/") && !uriPath.starts_with("/")) { + result.erase(0, 1); } + return result; } std::string Uri::uriPathToWindowsPath(const std::string& uriPath) { - // If the URI starts with `/c:` or similar, remove the initial slash. - size_t skip = 0; - if (uriPath.size() >= 3 && uriPath[0] == '/' && uriPath[1] != '/' && - uriPath[2] == ':') { - skip = 1; - } - - // UriParser docs: - // The destination buffer must be large enough to hold len(uriString) + 1 - // - 5 characters in case of an absolute URI or len(uriString) + 1 in case - // of a relative URI. - // However, the above seems to assume that uriPath starts with "file:", which - // is not required. - std::string result(uriPath.size() + 1, '\0'); - if (uriUriStringToWindowsFilenameA(uriPath.data() + skip, result.data()) != - 0) { - // Error - return original string. - return uriPath; - } else { - // Truncate at first null character - result.resize(std::strlen(result.data())); - return result; - } + std::string result = uriPathToUnixPath(uriPath); + conformWindowsPathInPlace(result); + return result; } -std::string Uri::uriPathToNativePath(const std::string& nativePath) { +std::string Uri::uriPathToNativePath(const std::string& uriPath) { #ifdef _WIN32 - return uriPathToWindowsPath(nativePath); + return uriPathToWindowsPath(uriPath); #else - return uriPathToUnixPath(nativePath); + return uriPathToUnixPath(uriPath); #endif } std::string Uri::getPath(const std::string& uri) { - UriUriA parsedUri; - if (uriParseSingleUriA(&parsedUri, uri.c_str(), nullptr) != URI_SUCCESS) { - // Could not parse the URI, so return an empty string. - return std::string(); - } - - // The initial string in this vector can be thought of as the "nothing" before - // the first slash in the path. - std::vector parts{std::string()}; - - UriPathSegmentA* pCurrent = parsedUri.pathHead; - while (pCurrent != nullptr) { - parts.emplace_back( - pCurrent->text.first, - size_t(pCurrent->text.afterLast - pCurrent->text.first)); - pCurrent = pCurrent->next; + UrlResult result = parseUrlConform(uri, true); + if (!result) { + return ""; } - uriFreeUriMembersA(&parsedUri); - - return joinToString(parts, "/"); + std::string path(result->get_pathname()); + return path; } std::string Uri::setPath(const std::string& uri, const std::string& newPath) { - UriUriA parsedUri; - if (uriParseSingleUriA(&parsedUri, uri.c_str(), nullptr) != URI_SUCCESS) { - // Could not parse the URI, so return an empty string. - return std::string(); - } - - // Free the existing path. Strangely, uriparser doesn't provide any simple way - // to do this. - UriPathSegmentA* pCurrent = parsedUri.pathHead; - while (pCurrent != nullptr) { - UriPathSegmentA* pNext = pCurrent->next; - free(pCurrent); - pCurrent = pNext; - } - - parsedUri.pathHead = nullptr; - parsedUri.pathTail = nullptr; - - // Set the new path. - if (!newPath.empty()) { - std::string::size_type startPos = 0; - do { - std::string::size_type nextSlashIndex = newPath.find('/', startPos); - - // Skip the initial slash if there is one. - if (nextSlashIndex == 0) { - startPos = 1; - continue; - } - - UriPathSegmentA* pSegment = - static_cast(malloc(sizeof(UriPathSegmentA))); - memset(pSegment, 0, sizeof(UriPathSegmentA)); - - if (parsedUri.pathHead == nullptr) { - parsedUri.pathHead = pSegment; - parsedUri.pathTail = pSegment; - } else { - parsedUri.pathTail->next = pSegment; - parsedUri.pathTail = parsedUri.pathTail->next; - } - - pSegment->text.first = newPath.data() + startPos; - - if (nextSlashIndex != std::string::npos) { - pSegment->text.afterLast = newPath.data() + nextSlashIndex; - startPos = nextSlashIndex + 1; - } else { - pSegment->text.afterLast = newPath.data() + newPath.size(); - startPos = nextSlashIndex; - } - } while (startPos != std::string::npos); - } - - int charsRequired; - if (uriToStringCharsRequiredA(&parsedUri, &charsRequired) != URI_SUCCESS) { - uriFreeUriMembersA(&parsedUri); - return uri; - } - - std::string result(static_cast(charsRequired), ' '); - - if (uriToStringA( - const_cast(result.c_str()), - &parsedUri, - charsRequired + 1, - nullptr) != URI_SUCCESS) { - uriFreeUriMembersA(&parsedUri); - return uri; + UrlResult result = parseUrlConform(uri, true); + if (!result) { + return ""; } - return result; + result->set_pathname(newPath); + return std::string(result->get_href()); } } // namespace CesiumUtility From 9617622deab15cc5961b2c7638a755d616619a58 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 12:07:24 -0500 Subject: [PATCH 02/28] Part of a Uri refactor --- .../test/TestLayerJsonTerrainLoader.cpp | 9 +- .../data/AddTileset/tileset3/tileset3.json | 2 +- CesiumUtility/include/CesiumUtility/Uri.h | 26 +++ CesiumUtility/src/Uri.cpp | 216 +++++++++++------- CesiumUtility/test/TestUri.cpp | 59 +++-- 5 files changed, 213 insertions(+), 99 deletions(-) diff --git a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp index 15520cc4e..eede5b441 100644 --- a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp @@ -261,7 +261,7 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.33.0"); CHECK( layers[0].tileTemplateUrls.front() == - "{z}/{x}/{y}.terrain?v={version}&extensions=octvertexnormals-metadata"); + "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-metadata"); CHECK(layers[0].loadedSubtrees.size() == 2); CHECK(layers[0].availabilityLevels == 10); } @@ -290,7 +290,7 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.0.0"); CHECK( layers[0].tileTemplateUrls.front() == - "{z}/{x}/{y}.terrain?v={version}&extensions=octvertexnormals"); + "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals"); CHECK(layers[0].loadedSubtrees.empty()); CHECK(layers[0].availabilityLevels == -1); @@ -313,7 +313,7 @@ TEST_CASE("Test create layer json terrain loader") { auto parentJsonPath = testDataPath / "CesiumTerrainTileJson" / "Parent.tile.json"; pMockedAssetAccessor->mockCompletedRequests.insert( - {"./Parent/layer.json", createMockAssetRequest(parentJsonPath)}); + {"Parent/layer.json", createMockAssetRequest(parentJsonPath)}); auto loaderFuture = LayerJsonTerrainLoader::createLoader(externals, {}, "layer.json", {}); @@ -411,8 +411,7 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].tileTemplateUrls.size() == 1); CHECK( layers[0].tileTemplateUrls[0] == - "{z}/{x}/" - "{y}.terrain?v={version}&extensions=octvertexnormals-watermask"); + "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-watermask"); } } diff --git a/Cesium3DTilesSelection/test/data/AddTileset/tileset3/tileset3.json b/Cesium3DTilesSelection/test/data/AddTileset/tileset3/tileset3.json index 1e0cd3663..5039c78d4 100644 --- a/Cesium3DTilesSelection/test/data/AddTileset/tileset3/tileset3.json +++ b/Cesium3DTilesSelection/test/data/AddTileset/tileset3/tileset3.json @@ -17,7 +17,7 @@ "geometricError": 0, "refine": "ADD", "content": { - "uri": "tileset3/ll.b3dm" + "uri": "ll.b3dm" } } } diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index c50bbb790..b1b924f3e 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -1,15 +1,36 @@ #pragma once #include +#include +#include #include +#include + +namespace ada { +struct url_aggregator; +struct url_search_params; +} namespace CesiumUtility { + /** * @brief A class for building and manipulating Uniform Resource Identifiers * (URIs). */ class Uri final { public: + Uri(const std::string& uri); + Uri(const Uri& uri); + Uri(const Uri& base, const std::string& relative, bool useBaseQuery = false); + + std::string toString() const; + bool isValid() const; + + const std::optional getQueryValue(const std::string& key) const; + void setQueryValue(const std::string& key, const std::string& value); + const std::string_view getPath() const; + void setPath(const std::string_view& path); + /** * @brief Attempts to resolve a relative URI using a base URI. * @@ -207,5 +228,10 @@ class Uri final { */ static std::string setPath(const std::string& uri, const std::string& newPath); + +private: + std::unique_ptr url = nullptr; + std::unique_ptr params = nullptr; + bool hasScheme = false; }; } // namespace CesiumUtility diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 7075d2b24..dcbe1dddb 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -38,94 +38,166 @@ void conformWindowsPathInPlace(std::string& path) { } } -std::string urlEncode(const std::string_view& value) { - std::ostringstream output; - output << std::hex; - output.fill('0'); - - for (size_t i = 0; i < value.length(); i++) { - // RFC 3986 unreserved characters - if (std::isalnum(value[i]) || value[i] == '-' || value[i] == '.' || - value[i] == '_' || value[i] == '~') { - output << value[i]; - } else { - output << std::uppercase; - output << '%'; - output << std::setw(2) << value[i]; - output << std::nouppercase; +constexpr const char* HTTPS_PREFIX = "https:"; +const std::string FILE_PREFIX = "file:///"; + +// C++ locale settings might change which values std::isalpha checks for. We only want ASCII. +bool isAsciiAlpha(char c) { + return c >= 0x41 && c <= 0x7a && (c <= 0x5a || c >= 0x61); +} +bool isAscii(char c) { return c <= 0x7f; } + +/** + * A URI has a valid scheme if it starts with an ASCII alpha character and has a sequence of ASCII characters followed by a "://" + */ +bool urlHasScheme(const std::string& uri) { + for (size_t i = 0; i < uri.length(); i++) { + if (uri[i] == ':') { + return uri.length() > i + 2 && uri [i + 1] == '/' && uri[i + 2] == '/'; + } else if (i == 0 && !isAsciiAlpha(uri[i])) { + // Scheme must start with an ASCII alpha character + return false; + } else if (!isAscii(uri[i])) { + // Scheme must be an ASCII string + return false; } } - return output.str(); + return false; } +} // namespace -const char* const HTTPS_PREFIX = "https:"; -const char* const HTTP_PREFIX = "http:"; +Uri::Uri(const std::string& uri) { + UrlResult result; + if (uri.starts_with("//")) { + // This is a protocol-relative URL. + // We will treat it as an HTTPS URL. + this->hasScheme = true; + result = ada::parse(HTTPS_PREFIX + uri); + } else { + this->hasScheme = urlHasScheme(uri); + result = this->hasScheme ? ada::parse(uri) : ada::parse(FILE_PREFIX + uri); + } -UrlResult parseUrlConform(const std::string& url, bool assumeHttpsDefault) { - if (url.starts_with("//")) { - return ada::parse( - std::string(assumeHttpsDefault ? HTTPS_PREFIX : HTTP_PREFIX) + url); + if (result) { + this->url = std::make_unique(std::move(result.value())); + this->params = + std::make_unique(this->url->get_search()); } - return ada::parse(url); } -UrlResult parseUrlConform(const std::string& url, const ada::url_aggregator* base, bool assumeHttpsDefault) { - if (url.starts_with("//")) { - return ada::parse( - std::string(assumeHttpsDefault ? HTTPS_PREFIX : HTTP_PREFIX) + url, base); +Uri::Uri(const Uri& uri) { + if (uri.url) { + this->url = std::make_unique(*uri.url); + this->params = + std::make_unique(this->url->get_search()); } - return ada::parse(url, base); + this->hasScheme = uri.hasScheme; } -} // namespace -std::string Uri::resolve( - const std::string& base, - const std::string& relative, - bool useBaseQuery, - [[maybe_unused]] bool assumeHttpsDefault) { - UrlResult baseUrl = parseUrlConform(base, assumeHttpsDefault); - if (!baseUrl) { - return relative; +Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { + UrlResult result; + if (!base.isValid()) { + this->hasScheme = urlHasScheme(relative); + result = this->hasScheme ? ada::parse(relative) + : ada::parse(FILE_PREFIX + relative); + } else { + this->hasScheme = base.hasScheme; + result = ada::parse(relative, base.url.get()); + } + + if (result) { + this->url = + std::make_unique(std::move(result.value())); + this->params = + std::make_unique(this->url->get_search()); + + if (useBaseQuery) { + // Set from relative to base to give priority to relative URL query string + for (const auto& [key, value] : *base.params) { + if (!this->params->has(key)) { + this->params->set(key, value); + } + } + this->url->set_search(this->params->to_string()); + } + } +} + +std::string Uri::toString() const { + if (!this->url) { + return ""; } - UrlResult relativeUrl = - parseUrlConform(relative, &baseUrl.value(), assumeHttpsDefault); - if (!relativeUrl) { - return relative; + // Update URL with any param modifications + this->url->set_search(this->params->to_string()); + const std::string_view result = this->url->get_href(); + return this->hasScheme ? std::string(result) + : std::string(result.substr(FILE_PREFIX.length())); +} + +bool Uri::isValid() const { + return this->url != nullptr && this->params != nullptr; } + +const std::optional +Uri::getQueryValue(const std::string& key) const { + if (!this->isValid()) { + return std::nullopt; } - if (useBaseQuery) { - relativeUrl->set_search(baseUrl->get_search()); + return this->params->get(key); +} + +void Uri::setQueryValue(const std::string& key, const std::string& value) { + if (!this->isValid()) { + return; } - return std::string(relativeUrl->get_href()); + this->params->set(key, value); +} + +const std::string_view Uri::getPath() const { + if (!this->isValid()) { + return {}; + } + + return this->url->get_pathname(); +} + +void Uri::setPath(const std::string_view& path) { + this->url->set_pathname(path); +} + +std::string Uri::resolve( + const std::string& base, + const std::string& relative, + bool useBaseQuery, + [[maybe_unused]] bool assumeHttpsDefault) { + return Uri(Uri(base), relative, useBaseQuery).toString(); } std::string Uri::addQuery( const std::string& uri, const std::string& key, const std::string& value) { - // TODO - if (uri.find('?') != std::string::npos) { - return uri + "&" + key + "=" + value; + Uri parsedUri(uri); + if (!parsedUri.isValid()) { + return uri; } - return uri + "?" + key + "=" + value; -} -std::string Uri::getQueryValue(const std::string& url, const std::string& key) { - UrlResult parsedUrl = parseUrlConform(url, true); - if (!parsedUrl) { - return ""; - } + parsedUri.setQueryValue(key, value); + return parsedUri.toString(); +} - ada::url_search_params params(parsedUrl->get_search()); - return std::string(params.get(key).value_or("")); +std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { + return std::string(Uri(uri).getQueryValue(key).value_or("")); } std::string Uri::substituteTemplateParameters( const std::string& templateUri, const std::function& substitutionCallback) { + const std::string& decodedUri = + ada::unicode::percent_decode(templateUri, templateUri.find('%')); std::string result; std::string placeholder; @@ -133,28 +205,28 @@ std::string Uri::substituteTemplateParameters( size_t nextPos; // Find the start of a parameter - while ((nextPos = templateUri.find('{', startPos)) != std::string::npos) { - result.append(templateUri, startPos, nextPos - startPos); + while ((nextPos = decodedUri.find('{', startPos)) != std::string::npos) { + result.append(decodedUri, startPos, nextPos - startPos); // Find the end of this parameter ++nextPos; - const size_t endPos = templateUri.find('}', nextPos); + const size_t endPos = decodedUri.find('}', nextPos); if (endPos == std::string::npos) { throw std::runtime_error("Unclosed template parameter"); } - placeholder = templateUri.substr(nextPos, endPos - nextPos); + placeholder = decodedUri.substr(nextPos, endPos - nextPos); result.append(substitutionCallback(placeholder)); startPos = endPos + 1; } - result.append(templateUri, startPos, templateUri.length() - startPos); + result.append(decodedUri, startPos, templateUri.length() - startPos); return result; } -std::string Uri::escape(const std::string& s) { return urlEncode(s); } +std::string Uri::escape(const std::string& s) { return ada::unicode::percent_encode(s, ada::character_sets::WWW_FORM_URLENCODED_PERCENT_ENCODE); } std::string Uri::unescape(const std::string& s) { return ada::unicode::percent_decode(s, s.find('%')); @@ -204,24 +276,12 @@ std::string Uri::uriPathToNativePath(const std::string& uriPath) { #endif } -std::string Uri::getPath(const std::string& uri) { - UrlResult result = parseUrlConform(uri, true); - if (!result) { - return ""; - } - - std::string path(result->get_pathname()); - return path; -} +std::string Uri::getPath(const std::string& uri) { return std::string(Uri(uri).getPath()); } std::string Uri::setPath(const std::string& uri, const std::string& newPath) { - UrlResult result = parseUrlConform(uri, true); - if (!result) { - return ""; - } - - result->set_pathname(newPath); - return std::string(result->get_href()); + Uri parsedUri(uri); + parsedUri.setPath(newPath); + return parsedUri.toString(); } } // namespace CesiumUtility diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 8d853c0a0..4349dea7f 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -21,23 +21,29 @@ TEST_CASE("Uri::getPath") { "/foo/bar/"); } - SECTION("returns empty path for nonexistent paths") { - CHECK(Uri::getPath("https://example.com") == ""); - CHECK(Uri::getPath("https://example.com?some=parameter") == ""); + SECTION("returns / path for nonexistent paths") { + CHECK(Uri::getPath("https://example.com") == "/"); + CHECK(Uri::getPath("https://example.com?some=parameter") == "/"); } - SECTION("returns empty path for invalid uri") { - CHECK(Uri::getPath("not a valid uri") == ""); + SECTION("handles unicode characters") { + CHECK(Uri::getPath("http://example.com/🐶.bin") == "/%F0%9F%90%B6.bin"); + CHECK(Uri::getPath("http://example.com/示例测试用例") == "/%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); + CHECK(Uri::getPath("http://example.com/Ῥόδος") == "/%E1%BF%AC%CF%8C%CE%B4%CE%BF%CF%82"); + CHECK( + Uri::getPath( + "http://example.com/🙍‍♂️🚪🤚/🪝🚗🚪/❓📞") == + "/%F0%9F%99%8D%E2%80%8D%E2%99%82%EF%B8%8F%F0%9F%9A%AA%F0%9F%A4%9A/%F0%9F%AA%9D%F0%9F%9A%97%F0%9F%9A%AA/%E2%9D%93%F0%9F%93%9E"); } } TEST_CASE("Uri::setPath") { SECTION("sets empty path") { - CHECK(Uri::setPath("https://example.com", "") == "https://example.com"); + CHECK(Uri::setPath("https://example.com/", "") == "https://example.com/"); } SECTION("sets new path") { - CHECK(Uri::setPath("https://example.com", "/") == "https://example.com/"); + CHECK(Uri::setPath("https://example.com/", "/") == "https://example.com/"); CHECK( Uri::setPath("https://example.com/foo", "/bar") == "https://example.com/bar"); @@ -52,7 +58,7 @@ TEST_CASE("Uri::setPath") { SECTION("preserves path parameters") { CHECK( Uri::setPath("https://example.com?some=parameter", "") == - "https://example.com?some=parameter"); + "https://example.com/?some=parameter"); CHECK( Uri::setPath("https://example.com?some=parameter", "/") == "https://example.com/?some=parameter"); @@ -77,8 +83,13 @@ TEST_CASE("Uri::setPath") { "/foo/bar") == "https://example.com/foo/bar?some=parameter"); } - SECTION("returns empty path for invalid uri") { - CHECK(Uri::setPath("not a valid uri", "/foo/") == ""); + SECTION("handles unicode characters") { + CHECK( + Uri::setPath("http://example.com/foo/", "/🐶.bin") == + "http://example.com/%F0%9F%90%B6.bin"); + CHECK( + Uri::setPath("http://example.com/bar/", "/示例测试用例") == + "http://example.com/%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); } } @@ -94,7 +105,10 @@ TEST_CASE("Uri::resolve") { "//www.example.com", "/page/test", false, - false) == "http://www.example.com/page/test"); + true) == "https://www.example.com/page/test"); + CHECK( + CesiumUtility::Uri::resolve("https://www.example.com/", "/Ῥόδος") == + "https://www.example.com/%E1%BF%AC%CF%8C%CE%B4%CE%BF%CF%82"); } TEST_CASE("Uri::escape") { @@ -114,7 +128,7 @@ TEST_CASE("Uri::unixPathToUriPath") { CHECK(Uri::unixPathToUriPath("wat") == "wat"); CHECK(Uri::unixPathToUriPath("wat/the") == "wat/the"); CHECK(Uri::unixPathToUriPath("/foo/bar") == "/foo/bar"); - CHECK(Uri::unixPathToUriPath("/some:file") == "/some%3Afile"); + CHECK(Uri::unixPathToUriPath("/some:file") == "/some:file"); CHECK(Uri::unixPathToUriPath("/🤞/😱/") == "/%F0%9F%A4%9E/%F0%9F%98%B1/"); } @@ -124,16 +138,16 @@ TEST_CASE("Uri::windowsPathToUriPath") { CHECK(Uri::windowsPathToUriPath("wat") == "wat"); CHECK(Uri::windowsPathToUriPath("/foo/bar") == "/foo/bar"); CHECK(Uri::windowsPathToUriPath("d:\\foo/bar\\") == "/d:/foo/bar/"); - CHECK(Uri::windowsPathToUriPath("e:\\some:file") == "/e:/some%3Afile"); + CHECK(Uri::windowsPathToUriPath("e:\\some:file") == "/e:/some:file"); CHECK( Uri::windowsPathToUriPath("c:/🤞/😱/") == "/c:/%F0%9F%A4%9E/%F0%9F%98%B1/"); CHECK( Uri::windowsPathToUriPath("notadriveletter:\\file") == - "notadriveletter%3A/file"); + "notadriveletter:/file"); CHECK( Uri::windowsPathToUriPath("\\notadriveletter:\\file") == - "/notadriveletter%3A/file"); + "/notadriveletter:/file"); } TEST_CASE("Uri::uriPathToUnixPath") { @@ -158,3 +172,18 @@ TEST_CASE("Uri::uriPathToWindowsPath") { Uri::uriPathToWindowsPath("/notadriveletter:/file") == "\\notadriveletter:\\file"); } + +TEST_CASE("Uri::addQuery") { + CHECK( + Uri::addQuery("https://example.com/", "a", "1") == + "https://example.com/?a=1"); + CHECK( + Uri::addQuery("https://example.com/?a=1", "b", "2") == + "https://example.com/?a=1&b=2"); + CHECK( + Uri::addQuery("https://example.com/?a=1", "a", "2") == + "https://example.com/?a=2"); + CHECK( + Uri::addQuery("https://unparseable url", "a", "1") == + "https://unparseable url"); +} From ad542c1dbf3bf006b27e84da3eb6867d536186b7 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 15:33:50 -0500 Subject: [PATCH 03/28] It's working!? --- CesiumUtility/src/Uri.cpp | 127 +++++++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 42 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index dcbe1dddb..b53404264 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include #include @@ -23,37 +23,26 @@ namespace CesiumUtility { using UrlResult = ada::result; namespace { -void conformWindowsPathInPlace(std::string& path) { - // Treated as a Unix path, c:/file.txt will be /c:/file.txt. - if (path.length() >= 3 && path[0] == '/' && std::isalpha(path[1]) && - path[2] == ':') { - path.erase(0, 1); - } - - // Replace slashes in-place - for (size_t i = 0; i < path.length(); i++) { - if (path[i] == '/') { - path[i] = '\\'; - } - } -} - constexpr const char* HTTPS_PREFIX = "https:"; const std::string FILE_PREFIX = "file:///"; +const char WINDOWS_PATH_SEP = '\\'; +const char PATH_SEP = '/'; -// C++ locale settings might change which values std::isalpha checks for. We only want ASCII. +// C++ locale settings might change which values std::isalpha checks for. We +// only want ASCII. bool isAsciiAlpha(char c) { return c >= 0x41 && c <= 0x7a && (c <= 0x5a || c >= 0x61); } bool isAscii(char c) { return c <= 0x7f; } /** - * A URI has a valid scheme if it starts with an ASCII alpha character and has a sequence of ASCII characters followed by a "://" + * A URI has a valid scheme if it starts with an ASCII alpha character and has a + * sequence of ASCII characters followed by a "://" */ bool urlHasScheme(const std::string& uri) { for (size_t i = 0; i < uri.length(); i++) { if (uri[i] == ':') { - return uri.length() > i + 2 && uri [i + 1] == '/' && uri[i + 2] == '/'; + return uri.length() > i + 2 && uri[i + 1] == '/' && uri[i + 2] == '/'; } else if (i == 0 && !isAsciiAlpha(uri[i])) { // Scheme must start with an ASCII alpha character return false; @@ -65,6 +54,20 @@ bool urlHasScheme(const std::string& uri) { return false; } + +class Path { +public: + Path(const std::string_view& buffer_) : buffer(buffer_) {} + + std::string toUriPath() const {} + + std::string toWindowsPath() const {} + + std::string toUnixPath() const {} + +private: + std::string buffer; +}; } // namespace Uri::Uri(const std::string& uri) { @@ -80,7 +83,8 @@ Uri::Uri(const std::string& uri) { } if (result) { - this->url = std::make_unique(std::move(result.value())); + this->url = + std::make_unique(std::move(result.value())); this->params = std::make_unique(this->url->get_search()); } @@ -137,7 +141,8 @@ std::string Uri::toString() const { } bool Uri::isValid() const { - return this->url != nullptr && this->params != nullptr; } + return this->url != nullptr && this->params != nullptr; +} const std::optional Uri::getQueryValue(const std::string& key) const { @@ -161,6 +166,7 @@ const std::string_view Uri::getPath() const { return {}; } + // Remove leading '/' return this->url->get_pathname(); } @@ -226,46 +232,81 @@ std::string Uri::substituteTemplateParameters( return result; } -std::string Uri::escape(const std::string& s) { return ada::unicode::percent_encode(s, ada::character_sets::WWW_FORM_URLENCODED_PERCENT_ENCODE); } +std::string Uri::escape(const std::string& s) { + return ada::unicode::percent_encode( + s, + ada::character_sets::WWW_FORM_URLENCODED_PERCENT_ENCODE); +} std::string Uri::unescape(const std::string& s) { return ada::unicode::percent_decode(s, s.find('%')); } std::string Uri::unixPathToUriPath(const std::string& unixPath) { - return nativePathToUriPath(unixPath); + return Uri::nativePathToUriPath(unixPath); } std::string Uri::windowsPathToUriPath(const std::string& windowsPath) { - return nativePathToUriPath(windowsPath); + return Uri::nativePathToUriPath(windowsPath); } std::string Uri::nativePathToUriPath(const std::string& nativePath) { - UrlResult parsedUrl = ada::parse("file://" + nativePath); - if (!parsedUrl) { - return nativePath; + const std::string encoded = ada::unicode::percent_encode( + nativePath, + ada::character_sets::PATH_PERCENT_ENCODE); + + const bool startsWithDriveLetter = + encoded.length() >= 2 && isAsciiAlpha(encoded[0]) && encoded[1] == ':'; + + std::string output; + output.reserve(encoded.length() + (startsWithDriveLetter ? 1 : 0)); + + // Paths like C:/... should be prefixed with a path separator + if (startsWithDriveLetter) { + output += PATH_SEP; } - std::string result(parsedUrl->get_pathname()); - return result; + // All we really need to do from here is convert our slashes + for (size_t i = 0; i < encoded.length(); i++) { + if (encoded[i] == WINDOWS_PATH_SEP) { + output += PATH_SEP; + } else { + output += encoded[i]; + } + } + + return output; } std::string Uri::uriPathToUnixPath(const std::string& uriPath) { - ada::url_aggregator url; - url.set_pathname(uriPath); - std::string result = ada::unicode::percent_decode( - url.get_pathname(), - url.get_pathname().find('%')); - if (result.starts_with("/") && !uriPath.starts_with("/")) { - result.erase(0, 1); - } - return result; + // URI paths are pretty much just unix paths with URL encoding + const std::string_view& rawPath = uriPath; + return ada::unicode::percent_decode(rawPath, rawPath.find('%')); } std::string Uri::uriPathToWindowsPath(const std::string& uriPath) { - std::string result = uriPathToUnixPath(uriPath); - conformWindowsPathInPlace(result); - return result; + const std::string path = + ada::unicode::percent_decode(uriPath, uriPath.find('%')); + + size_t i = 0; + // A path including a drive name will start like /C:/.... + // In that case, we just skip the first slash and continue on + if (path.length() >= 3 && path[0] == '/' && isAsciiAlpha(path[1]) && + path[2] == ':') { + i++; + } + + std::string output; + output.reserve(path.length() - i); + for (; i < path.length(); i++) { + if (path[i] == PATH_SEP) { + output += WINDOWS_PATH_SEP; + } else { + output += path[i]; + } + } + + return output; } std::string Uri::uriPathToNativePath(const std::string& uriPath) { @@ -276,7 +317,9 @@ std::string Uri::uriPathToNativePath(const std::string& uriPath) { #endif } -std::string Uri::getPath(const std::string& uri) { return std::string(Uri(uri).getPath()); } +std::string Uri::getPath(const std::string& uri) { + return std::string(Uri(uri).getPath()); +} std::string Uri::setPath(const std::string& uri, const std::string& newPath) { Uri parsedUri(uri); From 6fe2fdf51c74c435a1dc8cfd2bfe83d6a5a58b1b Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 16:02:32 -0500 Subject: [PATCH 04/28] Document methods --- CesiumUtility/include/CesiumUtility/Uri.h | 100 ++++++++++++++++++++-- CesiumUtility/test/TestUri.cpp | 3 + 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index b1b924f3e..5a4325516 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -9,26 +9,100 @@ namespace ada { struct url_aggregator; struct url_search_params; -} +} // namespace ada namespace CesiumUtility { /** - * @brief A class for building and manipulating Uniform Resource Identifiers + * @brief A class for parsing and manipulating Uniform Resource Identifiers * (URIs). + * + * The URI parser supports the [WhatWG URL + * specification](https://url.spec.whatwg.org/). It also supports + * protocol-relative URIs such as `//example.com`, and opaque paths such as + * `a/b/c`. */ class Uri final { public: + /** + * @brief Attempts to create a new Uri by parsing the given string. If the + * string fails to parse, \ref isValid will return false. + * + * @param uri A string containing the URI to attempt to parse. + */ Uri(const std::string& uri); - Uri(const Uri& uri); + /** + * @brief Attempts to create a new Uri from a base URI and a relative URI. If + * the base URI is invalid or the relative URI string fails to parse, \ref + * isValid will return false. + * + * @param base The base URI that the relative URI is relative to. + * @param relative A string containing a relative URI to attempt to parse. + * @param useBaseQuery If true, the resulting URI will include the query + * parameters of both the base URI and the relative URI. If false, only the + * relative URI's query parameters will be used (if any). + */ Uri(const Uri& base, const std::string& relative, bool useBaseQuery = false); + /** + * @brief Copy constructor for Uri. + */ + Uri(const Uri& uri); + + /** + * @brief Returns a string representation of the entire URI including path and + * query parameters. + */ std::string toString() const; + + /** + * @brief Returns true if this URI has been successfully parsed. + */ bool isValid() const; - const std::optional getQueryValue(const std::string& key) const; + /** + * @brief Equivalent to \ref isValid. + */ + operator bool() const { return this->isValid(); } + + /** + * @brief Obtains the value of the given key from the query string of the URI, + * if possible. + * + * If the URI can't be parsed, or the key doesn't exist in the + * query string, `std::nullopt` will be returned. + * + * @param key The key whose value will be obtained from the URI, if possible. + * @returns The value of the given key in the query string, or `std::nullopt` + * if not found. + */ + const std::optional + getQueryValue(const std::string& key) const; + + /** + * @brief Sets the given key in the URI's query parameters to the given value. + * If the key doesn't exist already, it will be added to the query parameters. + * Otherwise, the previous value will be overwritten. + * + * @param key The key to be added to the query string. + * @param value The value to be added to the query string. + */ void setQueryValue(const std::string& key, const std::string& value); + + /** + * @brief Gets the path portion of the URI. This will not include query + * parameters, if present. + * + * @return The path, or empty string if the URI could not be parsed. + */ const std::string_view getPath() const; + + /** + * @brief Sets the path portion of a URI to a new value. The other portions of + * the URI are left unmodified, including any query parameters. + * + * @param newPath The new path portion of the URI. + */ void setPath(const std::string_view& path); /** @@ -42,10 +116,11 @@ class Uri final { * @param relative The relative URI to be resolved against the base URI. * @param useBaseQuery If true, any query parameters of the base URI will be * retained in the resolved URI. - * @param assumeHttpsDefault If true, protocol-relative URIs (such as - * `//api.cesium.com`) will be assumed to be `https`. If false, they will be - * assumed to be `http`. + * @param assumeHttpsDefault This parameter is ignored and is only kept for + * API compatibility. * @returns The resolved URI. + * + * @deprecated Use the \ref Uri constructor instead. */ static std::string resolve( const std::string& base, @@ -61,6 +136,9 @@ class Uri final { * @param key The key to be added to the query string. * @param value The value to be added to the query string. * @returns The modified URI including the new query string parameter. + * + * @deprecated Create a \ref Uri instance and use \ref Uri::setQueryValue + * instead. */ static std::string addQuery( const std::string& uri, @@ -77,6 +155,9 @@ class Uri final { * @param key The key whose value will be obtained from the URI, if possible. * @returns The value of the given key in the query string, or an empty string * if not found. + * + * @deprecated Create a \ref Uri instance and use \ref + * Uri::getQueryValue(const std::string& key) instead. */ static std::string getQueryValue(const std::string& uri, const std::string& key); @@ -214,6 +295,8 @@ class Uri final { * * @param uri The URI from which to get the path. * @return The path, or empty string if the URI could not be parsed. + * + * @deprecated Create a \ref Uri instance and use \ref Uri::getPath() instead. */ static std::string getPath(const std::string& uri); @@ -225,6 +308,9 @@ class Uri final { * @param newPath The new path portion of the URI. * @returns The new URI after setting the path. If the original URI cannot be * parsed, it is returned unmodified. + * + * @deprecated Create a \ref Uri instance and use \ref Uri::setPath(const + * std::string_view& path) instead. */ static std::string setPath(const std::string& uri, const std::string& newPath); diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 4349dea7f..8c19e5134 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -186,4 +186,7 @@ TEST_CASE("Uri::addQuery") { CHECK( Uri::addQuery("https://unparseable url", "a", "1") == "https://unparseable url"); + CHECK( + Uri::addQuery("https://example.com/", "a", "!@#$%^&()_+{}|") == + "https://example.com/?a=%21%40%23%24%25%5E%26%28%29_%2B%7B%7D%7C"); } From f57f7d9687ef3a9b0acd66ce04dcb2b514c85b13 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 16:03:05 -0500 Subject: [PATCH 05/28] Remove cruft --- CesiumUtility/src/Uri.cpp | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index b53404264..50bf7d676 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -23,7 +23,7 @@ namespace CesiumUtility { using UrlResult = ada::result; namespace { -constexpr const char* HTTPS_PREFIX = "https:"; +const std::string HTTPS_PREFIX = "https:"; const std::string FILE_PREFIX = "file:///"; const char WINDOWS_PATH_SEP = '\\'; const char PATH_SEP = '/'; @@ -54,20 +54,6 @@ bool urlHasScheme(const std::string& uri) { return false; } - -class Path { -public: - Path(const std::string_view& buffer_) : buffer(buffer_) {} - - std::string toUriPath() const {} - - std::string toWindowsPath() const {} - - std::string toUnixPath() const {} - -private: - std::string buffer; -}; } // namespace Uri::Uri(const std::string& uri) { From 6115d11cf798bf78f85528c4790b5daaba8053a8 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 16:18:05 -0500 Subject: [PATCH 06/28] Format --- .../test/TestLayerJsonTerrainLoader.cpp | 10 +++++++--- CesiumUtility/test/TestUri.cpp | 16 +++++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp index d084d9f82..863f8b24a 100644 --- a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp @@ -261,7 +261,8 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.33.0"); CHECK( layers[0].tileTemplateUrls.front() == - "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-metadata"); + "%7Bz%7D/%7Bx%7D/" + "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-metadata"); CHECK(layers[0].loadedSubtrees.size() == 2); CHECK(layers[0].availabilityLevels == 10); } @@ -290,7 +291,8 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.0.0"); CHECK( layers[0].tileTemplateUrls.front() == - "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals"); + "%7Bz%7D/%7Bx%7D/" + "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals"); CHECK(layers[0].loadedSubtrees.empty()); CHECK(layers[0].availabilityLevels == -1); @@ -411,7 +413,9 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].tileTemplateUrls.size() == 1); CHECK( layers[0].tileTemplateUrls[0] == - "%7Bz%7D/%7Bx%7D/%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-watermask"); + "%7Bz%7D/%7Bx%7D/" + "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-" + "watermask"); } } diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 81b64c58f..67e67ce99 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -28,12 +28,17 @@ TEST_CASE("Uri::getPath") { SUBCASE("handles unicode characters") { CHECK(Uri::getPath("http://example.com/🐶.bin") == "/%F0%9F%90%B6.bin"); - CHECK(Uri::getPath("http://example.com/示例测试用例") == "/%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); - CHECK(Uri::getPath("http://example.com/Ῥόδος") == "/%E1%BF%AC%CF%8C%CE%B4%CE%BF%CF%82"); + CHECK( + Uri::getPath("http://example.com/示例测试用例") == + "/%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); + CHECK( + Uri::getPath("http://example.com/Ῥόδος") == + "/%E1%BF%AC%CF%8C%CE%B4%CE%BF%CF%82"); CHECK( Uri::getPath( "http://example.com/🙍‍♂️🚪🤚/🪝🚗🚪/❓📞") == - "/%F0%9F%99%8D%E2%80%8D%E2%99%82%EF%B8%8F%F0%9F%9A%AA%F0%9F%A4%9A/%F0%9F%AA%9D%F0%9F%9A%97%F0%9F%9A%AA/%E2%9D%93%F0%9F%93%9E"); + "/%F0%9F%99%8D%E2%80%8D%E2%99%82%EF%B8%8F%F0%9F%9A%AA%F0%9F%A4%9A/" + "%F0%9F%AA%9D%F0%9F%9A%97%F0%9F%9A%AA/%E2%9D%93%F0%9F%93%9E"); } } @@ -89,7 +94,8 @@ TEST_CASE("Uri::setPath") { "http://example.com/%F0%9F%90%B6.bin"); CHECK( Uri::setPath("http://example.com/bar/", "/示例测试用例") == - "http://example.com/%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); + "http://example.com/" + "%E7%A4%BA%E4%BE%8B%E6%B5%8B%E8%AF%95%E7%94%A8%E4%BE%8B"); } } @@ -188,5 +194,5 @@ TEST_CASE("Uri::addQuery") { "https://unparseable url"); CHECK( Uri::addQuery("https://example.com/", "a", "!@#$%^&()_+{}|") == - "https://example.com/?a=%21%40%23%24%25%5E%26%28%29_%2B%7B%7D%7C"); + "https://example.com/?a=%21%40%23%24%25%5E%26%28%29_%2B%7B%7D%7C"); } From eadf41997f7c379f992f9567b28834c301dd53ba Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 16:46:44 -0500 Subject: [PATCH 07/28] Fix clang-tidy issues --- CesiumUtility/src/Uri.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 50bf7d676..d2201dea8 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -1,22 +1,18 @@ #include -#include #include -#include -#include +#include +#include #include #include -#include #include #include -#include #include -#include -#include +#include #include #include -#include +#include namespace CesiumUtility { @@ -43,11 +39,8 @@ bool urlHasScheme(const std::string& uri) { for (size_t i = 0; i < uri.length(); i++) { if (uri[i] == ':') { return uri.length() > i + 2 && uri[i + 1] == '/' && uri[i + 2] == '/'; - } else if (i == 0 && !isAsciiAlpha(uri[i])) { - // Scheme must start with an ASCII alpha character - return false; - } else if (!isAscii(uri[i])) { - // Scheme must be an ASCII string + } else if ((i == 0 && !isAsciiAlpha(uri[i])) || !isAscii(uri[i])) { + // Scheme must start with an ASCII alpha character and be an ASCII string return false; } } From 887f9816a72b7f98ead8bd655a760b94e6d0df2c Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 17 Jan 2025 16:49:43 -0500 Subject: [PATCH 08/28] Fix Doxygen issues --- CesiumUtility/include/CesiumUtility/Uri.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 5a4325516..17bebafa8 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -101,7 +101,7 @@ class Uri final { * @brief Sets the path portion of a URI to a new value. The other portions of * the URI are left unmodified, including any query parameters. * - * @param newPath The new path portion of the URI. + * @param path The new path portion of the URI. */ void setPath(const std::string_view& path); @@ -157,7 +157,7 @@ class Uri final { * if not found. * * @deprecated Create a \ref Uri instance and use \ref - * Uri::getQueryValue(const std::string& key) instead. + * Uri::getQueryValue(const std::string&) instead. */ static std::string getQueryValue(const std::string& uri, const std::string& key); @@ -310,7 +310,7 @@ class Uri final { * parsed, it is returned unmodified. * * @deprecated Create a \ref Uri instance and use \ref Uri::setPath(const - * std::string_view& path) instead. + * std::string_view&) instead. */ static std::string setPath(const std::string& uri, const std::string& newPath); From 68aebad88104dc6dcf9294b2708717288142277e Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 20 Jan 2025 09:38:51 +1100 Subject: [PATCH 09/28] Remove find_package(uriparser) --- CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff43d56d4..0fb57e013 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -255,7 +255,6 @@ find_package(s2 CONFIG REQUIRED) find_package(spdlog CONFIG REQUIRED) find_package(tinyxml2 CONFIG REQUIRED) find_package(unofficial-sqlite3 CONFIG REQUIRED) -find_package(uriparser CONFIG REQUIRED char wchar_t) find_package(WebP CONFIG REQUIRED) find_package(ada CONFIG REQUIRED) From 78f0f40db194626d6aadad51fbd89fc141af03d4 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 20 Jan 2025 10:01:21 +1100 Subject: [PATCH 10/28] Re-add uriparser for now, and fix macOS compile error. --- CMakeLists.txt | 3 ++- CesiumUtility/include/CesiumUtility/Uri.h | 7 ++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0fb57e013..9a5592509 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,7 +45,7 @@ message(STATUS "VCPKG_OVERLAY_TRIPLETS ${VCPKG_OVERLAY_TRIPLETS}") # with the installation # Note that fmt is a public dependency of the vcpkg version of spdlog # STB and ada-url aren't technically part of the public interface, but they're used by the downstream Cesium for Unreal project -set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url) +set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url uriparser) # These packages are used in the code and produce binaries, but are not part of the public interface. Therefore we need # to distribute the binaries for linking, but not the headers, as downstream consumers don't need them # OpenSSL and abseil are both dependencies of s2geometry @@ -255,6 +255,7 @@ find_package(s2 CONFIG REQUIRED) find_package(spdlog CONFIG REQUIRED) find_package(tinyxml2 CONFIG REQUIRED) find_package(unofficial-sqlite3 CONFIG REQUIRED) +find_package(uriparser CONFIG REQUIRED char wchar_t) find_package(WebP CONFIG REQUIRED) find_package(ada CONFIG REQUIRED) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 17bebafa8..23dca51cb 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -1,16 +1,13 @@ #pragma once +#include + #include #include #include #include #include -namespace ada { -struct url_aggregator; -struct url_search_params; -} // namespace ada - namespace CesiumUtility { /** From e7774263e01a128a17653a95ac5f532fc833ab06 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 21 Jan 2025 13:33:48 -0500 Subject: [PATCH 11/28] Fix CI issues --- CesiumUtility/include/CesiumUtility/Uri.h | 3 +-- CesiumUtility/src/Uri.cpp | 17 ++++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 23dca51cb..543160a86 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -153,8 +153,7 @@ class Uri final { * @returns The value of the given key in the query string, or an empty string * if not found. * - * @deprecated Create a \ref Uri instance and use \ref - * Uri::getQueryValue(const std::string&) instead. + * @deprecated Create a \ref Uri instance andthe member function \ref getQueryValue instead. */ static std::string getQueryValue(const std::string& uri, const std::string& key); diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index d2201dea8..3690b97f6 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -1,18 +1,20 @@ #include -#include #include #include #include #include +#include #include #include #include +#include #include #include #include #include +#include namespace CesiumUtility { @@ -26,10 +28,10 @@ const char PATH_SEP = '/'; // C++ locale settings might change which values std::isalpha checks for. We // only want ASCII. -bool isAsciiAlpha(char c) { +bool isAsciiAlpha(unsigned char c) { return c >= 0x41 && c <= 0x7a && (c <= 0x5a || c >= 0x61); } -bool isAscii(char c) { return c <= 0x7f; } +bool isAscii(unsigned char c) { return c <= 0x7f; } /** * A URI has a valid scheme if it starts with an ASCII alpha character and has a @@ -37,9 +39,10 @@ bool isAscii(char c) { return c <= 0x7f; } */ bool urlHasScheme(const std::string& uri) { for (size_t i = 0; i < uri.length(); i++) { - if (uri[i] == ':') { + unsigned char c = static_cast(uri[i]); + if (c == ':') { return uri.length() > i + 2 && uri[i + 1] == '/' && uri[i + 2] == '/'; - } else if ((i == 0 && !isAsciiAlpha(uri[i])) || !isAscii(uri[i])) { + } else if ((i == 0 && !isAsciiAlpha(c)) || !isAscii(c)) { // Scheme must start with an ASCII alpha character and be an ASCII string return false; } @@ -235,7 +238,7 @@ std::string Uri::nativePathToUriPath(const std::string& nativePath) { ada::character_sets::PATH_PERCENT_ENCODE); const bool startsWithDriveLetter = - encoded.length() >= 2 && isAsciiAlpha(encoded[0]) && encoded[1] == ':'; + encoded.length() >= 2 && isAsciiAlpha(static_cast(encoded[0])) && encoded[1] == ':'; std::string output; output.reserve(encoded.length() + (startsWithDriveLetter ? 1 : 0)); @@ -270,7 +273,7 @@ std::string Uri::uriPathToWindowsPath(const std::string& uriPath) { size_t i = 0; // A path including a drive name will start like /C:/.... // In that case, we just skip the first slash and continue on - if (path.length() >= 3 && path[0] == '/' && isAsciiAlpha(path[1]) && + if (path.length() >= 3 && path[0] == '/' && isAsciiAlpha(static_cast(path[1])) && path[2] == ':') { i++; } From 38c1e31c632c7a8173a9d823b4f0f3117452b4ca Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 21 Jan 2025 13:34:46 -0500 Subject: [PATCH 12/28] Format --- CesiumUtility/include/CesiumUtility/Uri.h | 3 ++- CesiumUtility/src/Uri.cpp | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 543160a86..4b90300e1 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -153,7 +153,8 @@ class Uri final { * @returns The value of the given key in the query string, or an empty string * if not found. * - * @deprecated Create a \ref Uri instance andthe member function \ref getQueryValue instead. + * @deprecated Create a \ref Uri instance andthe member function \ref + * getQueryValue instead. */ static std::string getQueryValue(const std::string& uri, const std::string& key); diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 3690b97f6..eb3fd30de 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -238,7 +238,8 @@ std::string Uri::nativePathToUriPath(const std::string& nativePath) { ada::character_sets::PATH_PERCENT_ENCODE); const bool startsWithDriveLetter = - encoded.length() >= 2 && isAsciiAlpha(static_cast(encoded[0])) && encoded[1] == ':'; + encoded.length() >= 2 && + isAsciiAlpha(static_cast(encoded[0])) && encoded[1] == ':'; std::string output; output.reserve(encoded.length() + (startsWithDriveLetter ? 1 : 0)); @@ -273,8 +274,8 @@ std::string Uri::uriPathToWindowsPath(const std::string& uriPath) { size_t i = 0; // A path including a drive name will start like /C:/.... // In that case, we just skip the first slash and continue on - if (path.length() >= 3 && path[0] == '/' && isAsciiAlpha(static_cast(path[1])) && - path[2] == ':') { + if (path.length() >= 3 && path[0] == '/' && + isAsciiAlpha(static_cast(path[1])) && path[2] == ':') { i++; } From ea46377b3a691230783aeafd0c1b7a7b8783052c Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 23 Jan 2025 11:42:05 -0500 Subject: [PATCH 13/28] Rewrite substituteTemplateParameters to work with encoded brackets --- CesiumUtility/src/Uri.cpp | 50 ++++++++++++++++++++++------------ CesiumUtility/test/TestUri.cpp | 13 +++++++++ 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index eb3fd30de..76f701aa2 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -184,32 +184,48 @@ std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { std::string Uri::substituteTemplateParameters( const std::string& templateUri, const std::function& substitutionCallback) { - const std::string& decodedUri = - ada::unicode::percent_decode(templateUri, templateUri.find('%')); std::string result; + result.reserve(templateUri.length()); std::string placeholder; + bool inPlaceholder = false; size_t startPos = 0; - size_t nextPos; - - // Find the start of a parameter - while ((nextPos = decodedUri.find('{', startPos)) != std::string::npos) { - result.append(decodedUri, startPos, nextPos - startPos); - - // Find the end of this parameter - ++nextPos; - const size_t endPos = decodedUri.find('}', nextPos); - if (endPos == std::string::npos) { - throw std::runtime_error("Unclosed template parameter"); + size_t currentPos = 0; + + // Iterate through the string, replacing placeholders where found + while (currentPos < templateUri.length()) { + const bool roomForEncodedChar = currentPos < templateUri.length() - 2; + if(!inPlaceholder && ( + templateUri[currentPos] == '{' || + (roomForEncodedChar && templateUri[currentPos] == '%' && templateUri[currentPos + 1] == '7' && (templateUri[currentPos + 2] == 'B' || templateUri[currentPos + 2] == 'b')))) { + inPlaceholder = true; + startPos = currentPos + 1; + // Skip past rest of encoded char if necessary + if(templateUri[currentPos] == '%') { + currentPos += 2; + } + } else if(inPlaceholder && (templateUri[currentPos] == '}' || + (roomForEncodedChar && templateUri[currentPos] == '%' && templateUri[currentPos + 1] == '7' && (templateUri[currentPos + 2] == 'D' || templateUri[currentPos + 2] == 'd')))) { + placeholder = templateUri.substr(startPos, currentPos - startPos); + result.append(substitutionCallback(placeholder)); + inPlaceholder = false; + // Skip past rest of encoded char if necessary + if(templateUri[currentPos] == '%') { + currentPos += 2; + } + } else if(!inPlaceholder) { + result += templateUri[currentPos]; } - placeholder = decodedUri.substr(nextPos, endPos - nextPos); - result.append(substitutionCallback(placeholder)); + ++currentPos; + } - startPos = endPos + 1; + if(inPlaceholder) { + throw std::runtime_error("Unclosed template parameter"); } - result.append(decodedUri, startPos, templateUri.length() - startPos); + // It's possible some placeholders were replaced with strings shorter than the placeholder itself, so we might need to shrink to fit + result.shrink_to_fit(); return result; } diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 67e67ce99..87122247c 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -196,3 +196,16 @@ TEST_CASE("Uri::addQuery") { Uri::addQuery("https://example.com/", "a", "!@#$%^&()_+{}|") == "https://example.com/?a=%21%40%23%24%25%5E%26%28%29_%2B%7B%7D%7C"); } + +TEST_CASE("Uri::substituteTemplateParameters") { + CHECK( + Uri::substituteTemplateParameters("https://example.com/{a}/{b}/{c}", [](const std::string& placeholder) { return placeholder; }) == "https://example.com/a/b/c" + ); + CHECK( + Uri::substituteTemplateParameters("https://example.com/%7Ba%7D/test", []([[maybe_unused]] const std::string& placeholder) { return "1"; }) == "https://example.com/1/test" + ); + CHECK( + Uri::substituteTemplateParameters("https://example.com/enco%24d%5Ee%2Fd%7Bs%7Dtr1n%25g", []([[maybe_unused]] const std::string& placeholder) { return "teststr"; }) == "https://example.com/enco%24d%5Ee%2Fdteststrtr1n%25g" + ); + CHECK_THROWS_WITH_AS(Uri::substituteTemplateParameters("https://example.com/{unclosed placeholder", [](const std::string& placeholder) { return placeholder; }), "Unclosed template parameter", std::runtime_error); +} \ No newline at end of file From 605ea5a59ba039635c7b5ef8493ea3228629e47b Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 23 Jan 2025 11:46:11 -0500 Subject: [PATCH 14/28] Format --- CesiumUtility/src/Uri.cpp | 29 +++++++++++++++++++---------- CesiumUtility/test/TestUri.cpp | 31 ++++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 76f701aa2..5d82efbf2 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -195,36 +195,45 @@ std::string Uri::substituteTemplateParameters( // Iterate through the string, replacing placeholders where found while (currentPos < templateUri.length()) { const bool roomForEncodedChar = currentPos < templateUri.length() - 2; - if(!inPlaceholder && ( - templateUri[currentPos] == '{' || - (roomForEncodedChar && templateUri[currentPos] == '%' && templateUri[currentPos + 1] == '7' && (templateUri[currentPos + 2] == 'B' || templateUri[currentPos + 2] == 'b')))) { + if (!inPlaceholder && + (templateUri[currentPos] == '{' || + (roomForEncodedChar && templateUri[currentPos] == '%' && + templateUri[currentPos + 1] == '7' && + (templateUri[currentPos + 2] == 'B' || + templateUri[currentPos + 2] == 'b')))) { inPlaceholder = true; startPos = currentPos + 1; // Skip past rest of encoded char if necessary - if(templateUri[currentPos] == '%') { + if (templateUri[currentPos] == '%') { currentPos += 2; } - } else if(inPlaceholder && (templateUri[currentPos] == '}' || - (roomForEncodedChar && templateUri[currentPos] == '%' && templateUri[currentPos + 1] == '7' && (templateUri[currentPos + 2] == 'D' || templateUri[currentPos + 2] == 'd')))) { + } else if ( + inPlaceholder && + (templateUri[currentPos] == '}' || + (roomForEncodedChar && templateUri[currentPos] == '%' && + templateUri[currentPos + 1] == '7' && + (templateUri[currentPos + 2] == 'D' || + templateUri[currentPos + 2] == 'd')))) { placeholder = templateUri.substr(startPos, currentPos - startPos); result.append(substitutionCallback(placeholder)); inPlaceholder = false; // Skip past rest of encoded char if necessary - if(templateUri[currentPos] == '%') { + if (templateUri[currentPos] == '%') { currentPos += 2; } - } else if(!inPlaceholder) { + } else if (!inPlaceholder) { result += templateUri[currentPos]; } ++currentPos; } - if(inPlaceholder) { + if (inPlaceholder) { throw std::runtime_error("Unclosed template parameter"); } - // It's possible some placeholders were replaced with strings shorter than the placeholder itself, so we might need to shrink to fit + // It's possible some placeholders were replaced with strings shorter than the + // placeholder itself, so we might need to shrink to fit result.shrink_to_fit(); return result; diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 87122247c..55d45cc22 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -199,13 +199,26 @@ TEST_CASE("Uri::addQuery") { TEST_CASE("Uri::substituteTemplateParameters") { CHECK( - Uri::substituteTemplateParameters("https://example.com/{a}/{b}/{c}", [](const std::string& placeholder) { return placeholder; }) == "https://example.com/a/b/c" - ); - CHECK( - Uri::substituteTemplateParameters("https://example.com/%7Ba%7D/test", []([[maybe_unused]] const std::string& placeholder) { return "1"; }) == "https://example.com/1/test" - ); - CHECK( - Uri::substituteTemplateParameters("https://example.com/enco%24d%5Ee%2Fd%7Bs%7Dtr1n%25g", []([[maybe_unused]] const std::string& placeholder) { return "teststr"; }) == "https://example.com/enco%24d%5Ee%2Fdteststrtr1n%25g" - ); - CHECK_THROWS_WITH_AS(Uri::substituteTemplateParameters("https://example.com/{unclosed placeholder", [](const std::string& placeholder) { return placeholder; }), "Unclosed template parameter", std::runtime_error); + Uri::substituteTemplateParameters( + "https://example.com/{a}/{b}/{c}", + [](const std::string& placeholder) { return placeholder; }) == + "https://example.com/a/b/c"); + CHECK( + Uri::substituteTemplateParameters( + "https://example.com/%7Ba%7D/test", + []([[maybe_unused]] const std::string& placeholder) { + return "1"; + }) == "https://example.com/1/test"); + CHECK( + Uri::substituteTemplateParameters( + "https://example.com/enco%24d%5Ee%2Fd%7Bs%7Dtr1n%25g", + []([[maybe_unused]] const std::string& placeholder) { + return "teststr"; + }) == "https://example.com/enco%24d%5Ee%2Fdteststrtr1n%25g"); + CHECK_THROWS_WITH_AS( + Uri::substituteTemplateParameters( + "https://example.com/{unclosed placeholder", + [](const std::string& placeholder) { return placeholder; }), + "Unclosed template parameter", + std::runtime_error); } \ No newline at end of file From 8c84337b6cea1600bb939b190ad2c909c982090f Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 24 Jan 2025 14:03:48 -0500 Subject: [PATCH 15/28] A few more tests for edge cases, regex implementation --- CesiumUtility/src/Uri.cpp | 70 ++++++++++++++++------------------ CesiumUtility/test/TestUri.cpp | 44 +++++++++++++++------ 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 5d82efbf2..f7cf11b98 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include #include @@ -25,6 +25,9 @@ const std::string HTTPS_PREFIX = "https:"; const std::string FILE_PREFIX = "file:///"; const char WINDOWS_PATH_SEP = '\\'; const char PATH_SEP = '/'; +const std::regex TEMPLATE_REGEX( + "(\\{|%7B)(.*?)(\\}|%7D)", + std::regex::icase | std::regex::optimize); // C++ locale settings might change which values std::isalpha checks for. We // only want ASCII. @@ -184,52 +187,43 @@ std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { std::string Uri::substituteTemplateParameters( const std::string& templateUri, const std::function& substitutionCallback) { + std::string result; + // The output string will *probably* be at least as long as the input string. + // If not, we shrink_to_fit later. result.reserve(templateUri.length()); std::string placeholder; - bool inPlaceholder = false; + std::sregex_iterator begin( + templateUri.begin(), + templateUri.end(), + TEMPLATE_REGEX), + end; + size_t startPos = 0; - size_t currentPos = 0; - - // Iterate through the string, replacing placeholders where found - while (currentPos < templateUri.length()) { - const bool roomForEncodedChar = currentPos < templateUri.length() - 2; - if (!inPlaceholder && - (templateUri[currentPos] == '{' || - (roomForEncodedChar && templateUri[currentPos] == '%' && - templateUri[currentPos + 1] == '7' && - (templateUri[currentPos + 2] == 'B' || - templateUri[currentPos + 2] == 'b')))) { - inPlaceholder = true; - startPos = currentPos + 1; - // Skip past rest of encoded char if necessary - if (templateUri[currentPos] == '%') { - currentPos += 2; - } - } else if ( - inPlaceholder && - (templateUri[currentPos] == '}' || - (roomForEncodedChar && templateUri[currentPos] == '%' && - templateUri[currentPos + 1] == '7' && - (templateUri[currentPos + 2] == 'D' || - templateUri[currentPos + 2] == 'd')))) { - placeholder = templateUri.substr(startPos, currentPos - startPos); - result.append(substitutionCallback(placeholder)); - inPlaceholder = false; - // Skip past rest of encoded char if necessary - if (templateUri[currentPos] == '%') { - currentPos += 2; + + for (auto i = begin; i != end; i++) { + if (i->ready()) { + const size_t position = static_cast(i->position(0)); + if (position > startPos) { + result.append(templateUri.substr(startPos, position - startPos)); } - } else if (!inPlaceholder) { - result += templateUri[currentPos]; - } - ++currentPos; + placeholder = templateUri.substr( + static_cast(i->position(2)), + static_cast(i->length(2))); + result += ada::unicode::percent_encode( + substitutionCallback(placeholder), + ada::character_sets::WWW_FORM_URLENCODED_PERCENT_ENCODE); + + startPos = position + static_cast(i->length(0)); + } } - if (inPlaceholder) { - throw std::runtime_error("Unclosed template parameter"); + // Append rest of URL if any remaining + if (startPos < templateUri.length() - 1) { + result.append( + templateUri.substr(startPos, templateUri.length() - startPos)); } // It's possible some placeholders were replaced with strings shorter than the diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index 55d45cc22..e76561ef0 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -2,6 +2,9 @@ #include +#include +#include + using namespace CesiumUtility; TEST_CASE("Uri::getPath") { @@ -198,27 +201,46 @@ TEST_CASE("Uri::addQuery") { } TEST_CASE("Uri::substituteTemplateParameters") { + const std::map params{ + {"a", "aValue"}, + {"b", "bValue"}, + {"c", "cValue"}, + {"s", "teststr"}, + {"one", "1"}}; + + const auto substitutionCallback = [¶ms](const std::string& placeholder) { + auto it = params.find(placeholder); + return it == params.end() ? placeholder : it->second; + }; + CHECK( Uri::substituteTemplateParameters( "https://example.com/{a}/{b}/{c}", - [](const std::string& placeholder) { return placeholder; }) == - "https://example.com/a/b/c"); + substitutionCallback) == "https://example.com/aValue/bValue/cValue"); CHECK( Uri::substituteTemplateParameters( - "https://example.com/%7Ba%7D/test", - []([[maybe_unused]] const std::string& placeholder) { - return "1"; - }) == "https://example.com/1/test"); + "https://example.com/%7Bone%7D/test", + substitutionCallback) == "https://example.com/1/test"); CHECK( Uri::substituteTemplateParameters( "https://example.com/enco%24d%5Ee%2Fd%7Bs%7Dtr1n%25g", []([[maybe_unused]] const std::string& placeholder) { return "teststr"; }) == "https://example.com/enco%24d%5Ee%2Fdteststrtr1n%25g"); - CHECK_THROWS_WITH_AS( + CHECK( + Uri::substituteTemplateParameters( + "https://example.com/{a", + substitutionCallback) == "https://example.com/{a"); + CHECK( + Uri::substituteTemplateParameters( + "https://example.com/{}", + substitutionCallback) == "https://example.com/"); + CHECK( + Uri::substituteTemplateParameters( + "https://example.com/%7Ba}", + substitutionCallback) == "https://example.com/aValue"); + CHECK( Uri::substituteTemplateParameters( - "https://example.com/{unclosed placeholder", - [](const std::string& placeholder) { return placeholder; }), - "Unclosed template parameter", - std::runtime_error); + "https://example.com/a}", + substitutionCallback) == "https://example.com/a}"); } \ No newline at end of file From 17eefa161fef5ac3c301221e1922fd46b119abc0 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Tue, 28 Jan 2025 21:12:12 +1100 Subject: [PATCH 16/28] Substitute placeholders before manipulating layer.json URLs. --- .../src/LayerJsonTerrainLoader.cpp | 20 ++++++++------- .../src/LayerJsonTerrainLoader.h | 2 ++ .../test/TestLayerJsonTerrainLoader.cpp | 25 +++++++++++-------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp index f90a5bb7f..2355f115b 100644 --- a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp @@ -340,13 +340,6 @@ Future loadLayersRecursive( std::string extensionsToRequest = createExtensionsQueryParameter(knownExtensions, extensions); - if (!extensionsToRequest.empty()) { - for (std::string& url : urls) { - url = - CesiumUtility::Uri::addQuery(url, "extensions", extensionsToRequest); - } - } - const auto availabilityLevelsIt = layerJson.FindMember("metadataAvailability"); @@ -379,6 +372,7 @@ Future loadLayersRecursive( baseUrl, std::move(version), std::move(urls), + std::move(extensionsToRequest), std::move(availability), static_cast(maxZoom), availabilityLevels); @@ -648,12 +642,14 @@ LayerJsonTerrainLoader::Layer::Layer( const std::string& baseUrl_, std::string&& version_, std::vector&& tileTemplateUrls_, + std::string&& extensionsToRequest_, CesiumGeometry::QuadtreeRectangleAvailability&& contentAvailability_, uint32_t maxZooms_, int32_t availabilityLevels_) : baseUrl{baseUrl_}, version{std::move(version_)}, tileTemplateUrls{std::move(tileTemplateUrls_)}, + extensionsToRequest{std::move(extensionsToRequest_)}, contentAvailability{std::move(contentAvailability_)}, loadedSubtrees(maxSubtreeInLayer(maxZooms_, availabilityLevels_)), availabilityLevels{availabilityLevels_} {} @@ -675,9 +671,9 @@ std::string resolveTileUrl( return std::string(); } - return CesiumUtility::Uri::resolve( + Uri uri( layer.baseUrl, - CesiumUtility::Uri::substituteTemplateParameters( + Uri::substituteTemplateParameters( layer.tileTemplateUrls[0], [&tileID, &layer](const std::string& placeholder) -> std::string { if (placeholder == "level" || placeholder == "z") { @@ -695,6 +691,12 @@ std::string resolveTileUrl( return placeholder; })); + + if (!layer.extensionsToRequest.empty()) { + uri.setQueryValue("extensions", layer.extensionsToRequest); + } + + return uri.toString(); } Future requestTileContent( diff --git a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.h b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.h index a60900337..0f67bc596 100644 --- a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.h +++ b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.h @@ -52,6 +52,7 @@ class LayerJsonTerrainLoader : public TilesetContentLoader { const std::string& baseUrl, std::string&& version, std::vector&& tileTemplateUrls, + std::string&& extensionsToRequest, CesiumGeometry::QuadtreeRectangleAvailability&& contentAvailability, uint32_t maxZooms, int32_t availabilityLevels); @@ -59,6 +60,7 @@ class LayerJsonTerrainLoader : public TilesetContentLoader { std::string baseUrl; std::string version; std::vector tileTemplateUrls; + std::string extensionsToRequest; CesiumGeometry::QuadtreeRectangleAvailability contentAvailability; std::vector> loadedSubtrees; int32_t availabilityLevels; diff --git a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp index 863f8b24a..80116bbc4 100644 --- a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp @@ -261,8 +261,8 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.33.0"); CHECK( layers[0].tileTemplateUrls.front() == - "%7Bz%7D/%7Bx%7D/" - "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-metadata"); + "{z}/{x}/{y}.terrain?v={version}"); + CHECK(layers[0].extensionsToRequest == "octvertexnormals-metadata"); CHECK(layers[0].loadedSubtrees.size() == 2); CHECK(layers[0].availabilityLevels == 10); } @@ -291,8 +291,8 @@ TEST_CASE("Test create layer json terrain loader") { CHECK(layers[0].version == "1.0.0"); CHECK( layers[0].tileTemplateUrls.front() == - "%7Bz%7D/%7Bx%7D/" - "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals"); + "{z}/{x}/{y}.terrain?v={version}"); + CHECK(layers[0].extensionsToRequest == "octvertexnormals"); CHECK(layers[0].loadedSubtrees.empty()); CHECK(layers[0].availabilityLevels == -1); @@ -411,11 +411,8 @@ TEST_CASE("Test create layer json terrain loader") { const auto& layers = loaderResult.pLoader->getLayers(); CHECK(layers.size() == 1); CHECK(layers[0].tileTemplateUrls.size() == 1); - CHECK( - layers[0].tileTemplateUrls[0] == - "%7Bz%7D/%7Bx%7D/" - "%7By%7D.terrain?v=%7Bversion%7D&extensions=octvertexnormals-" - "watermask"); + CHECK(layers[0].tileTemplateUrls[0] == "{z}/{x}/{y}.terrain?v={version}"); + CHECK(layers[0].extensionsToRequest == "octvertexnormals-watermask"); } } @@ -446,6 +443,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}.terrain"}, + "one-two", std::move(contentAvailability), maxZoom, 10); @@ -454,7 +452,7 @@ TEST_CASE("Test load layer json tile content") { // mock tile content request pMockedAssetAccessor->mockCompletedRequests.insert( - {"0.0.0/1.0.0.terrain", + {"0.0.0/1.0.0.terrain?extensions=one-two", createMockAssetRequest( testDataPath / "CesiumTerrainTileJson" / "tile.metadataavailability.terrain")}); @@ -496,6 +494,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}.terrain"}, + std::string(), std::move(contentAvailability), maxZoom, -1); @@ -557,6 +556,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer0.terrain"}, + std::string(), std::move(layer0ContentAvailability), maxZoom, -1); @@ -571,6 +571,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer1.terrain"}, + std::string(), std::move(layer1ContentAvailability), maxZoom, -1); @@ -633,6 +634,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer0.terrain"}, + std::string(), std::move(layer0ContentAvailability), maxZoom, 10); @@ -644,6 +646,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer1.terrain"}, + std::string(), std::move(layer1ContentAvailability), maxZoom, 10); @@ -739,6 +742,7 @@ TEST_CASE("Test creating tile children for layer json") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer0.terrain"}, + std::string(), std::move(layer0ContentAvailability), maxZoom, 10); @@ -754,6 +758,7 @@ TEST_CASE("Test creating tile children for layer json") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}_layer1.terrain"}, + std::string(), std::move(layer1ContentAvailability), maxZoom, 10); From 97899114925871e42a72fe546befecd413251ac1 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 28 Jan 2025 10:12:11 -0500 Subject: [PATCH 17/28] Address most of review --- CMakeLists.txt | 2 +- CesiumUtility/include/CesiumUtility/Uri.h | 11 ++-- CesiumUtility/src/Uri.cpp | 80 ++++++++++------------- 3 files changed, 41 insertions(+), 52 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a5592509..ff43d56d4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,7 +45,7 @@ message(STATUS "VCPKG_OVERLAY_TRIPLETS ${VCPKG_OVERLAY_TRIPLETS}") # with the installation # Note that fmt is a public dependency of the vcpkg version of spdlog # STB and ada-url aren't technically part of the public interface, but they're used by the downstream Cesium for Unreal project -set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url uriparser) +set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url) # These packages are used in the code and produce binaries, but are not part of the public interface. Therefore we need # to distribute the binaries for linking, but not the headers, as downstream consumers don't need them # OpenSSL and abseil are both dependencies of s2geometry diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 4b90300e1..3a83c91ab 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -74,7 +73,7 @@ class Uri final { * if not found. */ const std::optional - getQueryValue(const std::string& key) const; + getQueryValue(const std::string& key); /** * @brief Sets the given key in the URI's query parameters to the given value. @@ -92,7 +91,7 @@ class Uri final { * * @return The path, or empty string if the URI could not be parsed. */ - const std::string_view getPath() const; + std::string_view getPath() const; /** * @brief Sets the path portion of a URI to a new value. The other portions of @@ -313,8 +312,8 @@ class Uri final { setPath(const std::string& uri, const std::string& newPath); private: - std::unique_ptr url = nullptr; - std::unique_ptr params = nullptr; - bool hasScheme = false; + std::optional _url = std::nullopt; + std::optional _params = std::nullopt; + bool _hasScheme = false; }; } // namespace CesiumUtility diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index f7cf11b98..0079537d7 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -18,16 +18,16 @@ namespace CesiumUtility { -using UrlResult = ada::result; - namespace { const std::string HTTPS_PREFIX = "https:"; const std::string FILE_PREFIX = "file:///"; const char WINDOWS_PATH_SEP = '\\'; const char PATH_SEP = '/'; + const std::regex TEMPLATE_REGEX( - "(\\{|%7B)(.*?)(\\}|%7D)", - std::regex::icase | std::regex::optimize); + "\\{(.*?)\\}", std::regex::optimize); + +using UrlResult = ada::result; // C++ locale settings might change which values std::isalpha checks for. We // only want ASCII. @@ -60,82 +60,75 @@ Uri::Uri(const std::string& uri) { if (uri.starts_with("//")) { // This is a protocol-relative URL. // We will treat it as an HTTPS URL. - this->hasScheme = true; + this->_hasScheme = true; result = ada::parse(HTTPS_PREFIX + uri); } else { - this->hasScheme = urlHasScheme(uri); - result = this->hasScheme ? ada::parse(uri) : ada::parse(FILE_PREFIX + uri); + this->_hasScheme = urlHasScheme(uri); + result = this->_hasScheme ? ada::parse(uri) : ada::parse(FILE_PREFIX + uri); } if (result) { - this->url = - std::make_unique(std::move(result.value())); - this->params = - std::make_unique(this->url->get_search()); + this->_url.emplace(std::move(result.value())); + this->_params.emplace(this->_url->get_search()); } } Uri::Uri(const Uri& uri) { - if (uri.url) { - this->url = std::make_unique(*uri.url); - this->params = - std::make_unique(this->url->get_search()); + if (uri._url) { + this->_url.emplace(*uri._url); + this->_params.emplace(this->_url->get_search()); } - this->hasScheme = uri.hasScheme; + this->_hasScheme = uri._hasScheme; } Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { UrlResult result; if (!base.isValid()) { - this->hasScheme = urlHasScheme(relative); - result = this->hasScheme ? ada::parse(relative) + this->_hasScheme = urlHasScheme(relative); + result = this->_hasScheme ? ada::parse(relative) : ada::parse(FILE_PREFIX + relative); } else { - this->hasScheme = base.hasScheme; - result = ada::parse(relative, base.url.get()); + this->_hasScheme = base._hasScheme; + result = ada::parse(relative, &base._url.value()); } if (result) { - this->url = - std::make_unique(std::move(result.value())); - this->params = - std::make_unique(this->url->get_search()); + this->_url.emplace(std::move(result.value())); + this->_params.emplace(this->_url->get_search()); if (useBaseQuery) { // Set from relative to base to give priority to relative URL query string - for (const auto& [key, value] : *base.params) { - if (!this->params->has(key)) { - this->params->set(key, value); + for (const auto& [key, value] : *base._params) { + if (!this->_params->has(key)) { + this->_params->set(key, value); } } - this->url->set_search(this->params->to_string()); + this->_url->set_search(this->_params->to_string()); } } } std::string Uri::toString() const { - if (!this->url) { + if (!this->_url) { return ""; } - // Update URL with any param modifications - this->url->set_search(this->params->to_string()); - const std::string_view result = this->url->get_href(); - return this->hasScheme ? std::string(result) + const std::string_view result = this->_url->get_href(); + return this->_hasScheme ? std::string(result) : std::string(result.substr(FILE_PREFIX.length())); } bool Uri::isValid() const { - return this->url != nullptr && this->params != nullptr; + return this->_url && this->_params; } const std::optional -Uri::getQueryValue(const std::string& key) const { +Uri::getQueryValue(const std::string& key) { if (!this->isValid()) { return std::nullopt; } - return this->params->get(key); + return this->_params->get(key); } void Uri::setQueryValue(const std::string& key, const std::string& value) { @@ -143,20 +136,22 @@ void Uri::setQueryValue(const std::string& key, const std::string& value) { return; } - this->params->set(key, value); + this->_params->set(key, value); + // Update URL with modified params + this->_url->set_search(this->_params->to_string()); } -const std::string_view Uri::getPath() const { +std::string_view Uri::getPath() const { if (!this->isValid()) { return {}; } // Remove leading '/' - return this->url->get_pathname(); + return this->_url->get_pathname(); } void Uri::setPath(const std::string_view& path) { - this->url->set_pathname(path); + this->_url->set_pathname(path); } std::string Uri::resolve( @@ -190,7 +185,6 @@ std::string Uri::substituteTemplateParameters( std::string result; // The output string will *probably* be at least as long as the input string. - // If not, we shrink_to_fit later. result.reserve(templateUri.length()); std::string placeholder; @@ -226,10 +220,6 @@ std::string Uri::substituteTemplateParameters( templateUri.substr(startPos, templateUri.length() - startPos)); } - // It's possible some placeholders were replaced with strings shorter than the - // placeholder itself, so we might need to shrink to fit - result.shrink_to_fit(); - return result; } From cb18f767b37073dbf159d45ac1f4591924e69b7d Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 28 Jan 2025 10:42:41 -0500 Subject: [PATCH 18/28] Return to previous substitution logic --- .../test/TestLayerJsonTerrainLoader.cpp | 1 + CesiumUtility/src/Uri.cpp | 52 +++++++------------ CesiumUtility/test/TestUri.cpp | 10 +--- 3 files changed, 20 insertions(+), 43 deletions(-) diff --git a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp index 5a7afcf00..d8b0877fc 100644 --- a/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/test/TestLayerJsonTerrainLoader.cpp @@ -730,6 +730,7 @@ TEST_CASE("Test load layer json tile content") { "layer.json", "1.0.0", std::vector{"{level}.{x}.{y}/{version}.terrain"}, + std::string{}, std::move(contentAvailability), maxZoom, 10); diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 0079537d7..6840fe81a 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -9,9 +9,7 @@ #include #include #include -#include #include -#include #include #include #include @@ -24,9 +22,6 @@ const std::string FILE_PREFIX = "file:///"; const char WINDOWS_PATH_SEP = '\\'; const char PATH_SEP = '/'; -const std::regex TEMPLATE_REGEX( - "\\{(.*?)\\}", std::regex::optimize); - using UrlResult = ada::result; // C++ locale settings might change which values std::isalpha checks for. We @@ -182,43 +177,32 @@ std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { std::string Uri::substituteTemplateParameters( const std::string& templateUri, const std::function& substitutionCallback) { - std::string result; - // The output string will *probably* be at least as long as the input string. - result.reserve(templateUri.length()); std::string placeholder; - std::sregex_iterator begin( - templateUri.begin(), - templateUri.end(), - TEMPLATE_REGEX), - end; - size_t startPos = 0; + size_t nextPos; + + // Find the start of a parameter + while ((nextPos = templateUri.find('{', startPos)) != std::string::npos) { + result.append(templateUri, startPos, nextPos - startPos); + + // Find the end of this parameter + ++nextPos; + const size_t endPos = templateUri.find('}', nextPos); + if (endPos == std::string::npos) { + // It's not a properly closed placeholder, so let's just output the rest of the URL (including the open brace) as-is and bail. + startPos = nextPos - 1; + break; + } - for (auto i = begin; i != end; i++) { - if (i->ready()) { - const size_t position = static_cast(i->position(0)); - if (position > startPos) { - result.append(templateUri.substr(startPos, position - startPos)); - } - - placeholder = templateUri.substr( - static_cast(i->position(2)), - static_cast(i->length(2))); - result += ada::unicode::percent_encode( - substitutionCallback(placeholder), - ada::character_sets::WWW_FORM_URLENCODED_PERCENT_ENCODE); + placeholder = templateUri.substr(nextPos, endPos - nextPos); + result.append(substitutionCallback(placeholder)); - startPos = position + static_cast(i->length(0)); - } + startPos = endPos + 1; } - // Append rest of URL if any remaining - if (startPos < templateUri.length() - 1) { - result.append( - templateUri.substr(startPos, templateUri.length() - startPos)); - } + result.append(templateUri, startPos, templateUri.length() - startPos); return result; } diff --git a/CesiumUtility/test/TestUri.cpp b/CesiumUtility/test/TestUri.cpp index e76561ef0..14f7631cc 100644 --- a/CesiumUtility/test/TestUri.cpp +++ b/CesiumUtility/test/TestUri.cpp @@ -219,11 +219,7 @@ TEST_CASE("Uri::substituteTemplateParameters") { substitutionCallback) == "https://example.com/aValue/bValue/cValue"); CHECK( Uri::substituteTemplateParameters( - "https://example.com/%7Bone%7D/test", - substitutionCallback) == "https://example.com/1/test"); - CHECK( - Uri::substituteTemplateParameters( - "https://example.com/enco%24d%5Ee%2Fd%7Bs%7Dtr1n%25g", + "https://example.com/enco%24d%5Ee%2Fd{s}tr1n%25g", []([[maybe_unused]] const std::string& placeholder) { return "teststr"; }) == "https://example.com/enco%24d%5Ee%2Fdteststrtr1n%25g"); @@ -235,10 +231,6 @@ TEST_CASE("Uri::substituteTemplateParameters") { Uri::substituteTemplateParameters( "https://example.com/{}", substitutionCallback) == "https://example.com/"); - CHECK( - Uri::substituteTemplateParameters( - "https://example.com/%7Ba}", - substitutionCallback) == "https://example.com/aValue"); CHECK( Uri::substituteTemplateParameters( "https://example.com/a}", From 7006b8177c458ee9bc4cea627884bab8d0525b65 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 28 Jan 2025 11:57:53 -0500 Subject: [PATCH 19/28] Remove uriparser, format, etc --- CHANGES.md | 1 + CMakeLists.txt | 1 - Cesium3DTilesSelection/CMakeLists.txt | 1 - CesiumIonClient/src/Connection.cpp | 21 ++++++-------- CesiumUtility/include/CesiumUtility/Uri.h | 18 ++++++++++-- CesiumUtility/src/Uri.cpp | 34 ++++++++++++++++++----- ThirdParty.json | 12 ++++---- doc/topics/dependencies.md | 2 +- 8 files changed, 60 insertions(+), 30 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 64b044f45..41c75737e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -24,6 +24,7 @@ - Fixed a bug in `SharedAssetDepot` that could cause assertion failures in debug builds, and could rarely cause premature deletion of shared assets even in release builds. - Fixed a bug that could cause `Tileset::sampleHeightMostDetailed` to return a height that is not the highest one when the sampled tileset contained multiple heights at the given location. - `LayerJsonTerrainLoader` will now log errors and warnings when failing to load a `.terrain` file referenced in the layer.json, instead of silently ignoring them. +- URIs containing unicode characters are now supported. ### v0.43.0 - 2025-01-02 diff --git a/CMakeLists.txt b/CMakeLists.txt index ff43d56d4..0fb57e013 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -255,7 +255,6 @@ find_package(s2 CONFIG REQUIRED) find_package(spdlog CONFIG REQUIRED) find_package(tinyxml2 CONFIG REQUIRED) find_package(unofficial-sqlite3 CONFIG REQUIRED) -find_package(uriparser CONFIG REQUIRED char wchar_t) find_package(WebP CONFIG REQUIRED) find_package(ada CONFIG REQUIRED) diff --git a/Cesium3DTilesSelection/CMakeLists.txt b/Cesium3DTilesSelection/CMakeLists.txt index ab5d18a39..a6e1588a3 100644 --- a/Cesium3DTilesSelection/CMakeLists.txt +++ b/Cesium3DTilesSelection/CMakeLists.txt @@ -54,7 +54,6 @@ target_link_libraries(Cesium3DTilesSelection CesiumUtility spdlog::spdlog spdlog::spdlog_header_only # PRIVATE - uriparser::uriparser libmorton::libmorton draco::draco nonstd::expected-lite diff --git a/CesiumIonClient/src/Connection.cpp b/CesiumIonClient/src/Connection.cpp index e7a1cc352..96f9ef313 100644 --- a/CesiumIonClient/src/Connection.cpp +++ b/CesiumIonClient/src/Connection.cpp @@ -29,8 +29,6 @@ #include #include #include -#include -#include #include #include @@ -495,19 +493,18 @@ Asset jsonToAsset(const rapidjson::Value& item) { } std::optional generateApiUrl(const std::string& ionUrl) { - UriUriA newUri; - if (uriParseSingleUriA(&newUri, ionUrl.c_str(), nullptr) != URI_SUCCESS) { - return std::optional(); + Uri parsedIonUrl(ionUrl); + if(!parsedIonUrl) { + return std::nullopt; } - std::string hostName = - std::string(newUri.hostText.first, newUri.hostText.afterLast); - std::string scheme = - std::string(newUri.scheme.first, newUri.scheme.afterLast); - - uriFreeUriMembersA(&newUri); + std::string url; + url.append(parsedIonUrl.getScheme()); + url.append("//api."); + url.append(parsedIonUrl.getHost()); + url.append("/"); - return std::make_optional(scheme + "://api." + hostName + '/'); + return url; } } // namespace diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 3a83c91ab..481ce637e 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -72,8 +72,7 @@ class Uri final { * @returns The value of the given key in the query string, or `std::nullopt` * if not found. */ - const std::optional - getQueryValue(const std::string& key); + const std::optional getQueryValue(const std::string& key); /** * @brief Sets the given key in the URI's query parameters to the given value. @@ -85,6 +84,21 @@ class Uri final { */ void setQueryValue(const std::string& key, const std::string& value); + /** + * @brief Gets the scheme portion of the URI. If the URI was created without a scheme, this will return an empty string. + * + * @returns The scheme, or an empty string if the URI could not be parsed or has no scheme. + */ + std::string_view getScheme() const; + + /** + * @brief Gets the host portion of the URI. If the URI also specifies a non-default port, it will be included in the returned host. + * If the URI contains no host, this will return an empty string. + * + * @returns The host, or an empty string if the URI could not be parsed or has no host. + */ + std::string_view getHost() const; + /** * @brief Gets the path portion of the URI. This will not include query * parameters, if present. diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 6840fe81a..2a903f2ec 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -50,6 +50,9 @@ bool urlHasScheme(const std::string& uri) { } } // namespace +// clang-tidy isn't smart enough to understand that we *are* checking the +// optionals before accessing them, so disable the warning. +// NOLINTBEGIN(bugprone-unchecked-optional-access) Uri::Uri(const std::string& uri) { UrlResult result; if (uri.starts_with("//")) { @@ -81,7 +84,7 @@ Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { if (!base.isValid()) { this->_hasScheme = urlHasScheme(relative); result = this->_hasScheme ? ada::parse(relative) - : ada::parse(FILE_PREFIX + relative); + : ada::parse(FILE_PREFIX + relative); } else { this->_hasScheme = base._hasScheme; result = ada::parse(relative, &base._url.value()); @@ -110,12 +113,10 @@ std::string Uri::toString() const { const std::string_view result = this->_url->get_href(); return this->_hasScheme ? std::string(result) - : std::string(result.substr(FILE_PREFIX.length())); + : std::string(result.substr(FILE_PREFIX.length())); } -bool Uri::isValid() const { - return this->_url && this->_params; -} +bool Uri::isValid() const { return this->_url && this->_params; } const std::optional Uri::getQueryValue(const std::string& key) { @@ -136,6 +137,22 @@ void Uri::setQueryValue(const std::string& key, const std::string& value) { this->_url->set_search(this->_params->to_string()); } +std::string_view Uri::getScheme() const { + if(!this->isValid()) { + return {}; + } + + return this->_hasScheme ? this->_url->get_protocol() : std::string_view{}; +} + +std::string_view Uri::getHost() const { + if(!this->isValid()) { + return {}; + } + + return this->_url->get_host(); +} + std::string_view Uri::getPath() const { if (!this->isValid()) { return {}; @@ -174,6 +191,8 @@ std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { return std::string(Uri(uri).getQueryValue(key).value_or("")); } +// NOLINTEND(bugprone-unchecked-optional-access) + std::string Uri::substituteTemplateParameters( const std::string& templateUri, const std::function& substitutionCallback) { @@ -191,7 +210,8 @@ std::string Uri::substituteTemplateParameters( ++nextPos; const size_t endPos = templateUri.find('}', nextPos); if (endPos == std::string::npos) { - // It's not a properly closed placeholder, so let's just output the rest of the URL (including the open brace) as-is and bail. + // It's not a properly closed placeholder, so let's just output the rest + // of the URL (including the open brace) as-is and bail. startPos = nextPos - 1; break; } @@ -303,4 +323,4 @@ std::string Uri::setPath(const std::string& uri, const std::string& newPath) { return parsedUri.toString(); } -} // namespace CesiumUtility +} // namespace CesiumUtility \ No newline at end of file diff --git a/ThirdParty.json b/ThirdParty.json index 7616d8505..32c52a42d 100644 --- a/ThirdParty.json +++ b/ThirdParty.json @@ -1,4 +1,10 @@ [ + { + "name": "ada", + "url": "https://github.com/ada-url/ada", + "version": "2.9.2", + "license": ["Apache 2.0"] + }, { "name": "Async++", "url": "https://github.com/Amanieu/asyncplusplus", @@ -114,12 +120,6 @@ "version": "1aeb57d26bc303d5cfa1a9ff2a331df7ba278656", "license": ["zlib"] }, - { - "name": "uriparser", - "url": "https://github.com/uriparser/uriparser", - "version": "0.9.6", - "license": ["BSD-3-Clause"] - }, { "name": "zlib", "url": "https://github.com/madler/zlib", diff --git a/doc/topics/dependencies.md b/doc/topics/dependencies.md index b3b418f23..9c026b04b 100644 --- a/doc/topics/dependencies.md +++ b/doc/topics/dependencies.md @@ -4,6 +4,7 @@ Cesium Native relies on a number of third-party dependencies. These dependencies | Dependency | Usage | | ------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| [ada](https://github.com/ada-url/ada) | Used to parse and manipulate URIs. | | [Async++](https://github.com/Amanieu/asyncplusplus) | Used by CesiumAsync for cross-platform concurrency. | | [Catch2](https://github.com/catchorg/Catch2) | Test framework used by CesiumNativeTests. | | [draco](https://github.com/google/draco) | Required to decode meshes and point clouds compressed with Draco. | @@ -25,7 +26,6 @@ Cesium Native relies on a number of third-party dependencies. These dependencies | [sqlite3](https://www.sqlite.org/index.html) | Used to cache HTTP responses. | | [stb_image](https://github.com/nothings/stb/blob/master/stb_image.h) | A simple image loader. | | [tinyxml2](https://github.com/leethomason/tinyxml2) | XML parser for interacting with XML APIs such as those implementing the Web Map Service standard. | -| [uriparser](https://github.com/uriparser/uriparser) | Used to parse and manipulate URIs. | | [zlib-ng](https://github.com/zlib-ng/zlib-ng) | An optimized zlib implementation for working with Gzipped data. | The following chart illustrates the connections between the Cesium Native libraries and third-party dependencies: From 3d5d39d7aa0bea9f5c1c23965a0284249e13b68f Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 28 Jan 2025 12:52:52 -0500 Subject: [PATCH 20/28] Update dependency graphs --- .../dependencies/Cesium3DTilesSelection.mmd | 3 +- doc/diagrams/dependencies/CesiumIonClient.mmd | 3 +- doc/diagrams/dependencies/CesiumUtility.mmd | 4 +-- doc/diagrams/dependencies/all.mmd | 36 +++++++++---------- doc/img/dependency-graph.svg | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/doc/diagrams/dependencies/Cesium3DTilesSelection.mmd b/doc/diagrams/dependencies/Cesium3DTilesSelection.mmd index 08bf7d913..88f68b564 100644 --- a/doc/diagrams/dependencies/Cesium3DTilesSelection.mmd +++ b/doc/diagrams/dependencies/Cesium3DTilesSelection.mmd @@ -23,6 +23,5 @@ graph TD Cesium3DTilesSelection[Cesium3DTilesSelection] --> spdlog_spdlog{{spdlog::spdlog}} Cesium3DTilesSelection[Cesium3DTilesSelection] --> spdlog_spdlog_header_only{{spdlog::spdlog_header_only}} Cesium3DTilesSelection[Cesium3DTilesSelection] --> tinyxml2_tinyxml2{{tinyxml2::tinyxml2}} - Cesium3DTilesSelection[Cesium3DTilesSelection] --> uriparser_uriparser{{uriparser::uriparser}} - class draco_draco,libmorton_libmorton,nonstd_expected-lite,spdlog_spdlog,spdlog_spdlog_header_only,tinyxml2_tinyxml2,uriparser_uriparser dependencyNode + class draco_draco,libmorton_libmorton,nonstd_expected-lite,spdlog_spdlog,spdlog_spdlog_header_only,tinyxml2_tinyxml2 dependencyNode class Cesium3DTiles,Cesium3DTilesContent,Cesium3DTilesReader,CesiumAsync,CesiumGeometry,CesiumGeospatial,CesiumGltf,CesiumGltfReader,CesiumQuantizedMeshTerrain,CesiumRasterOverlays,CesiumUtility,Cesium3DTilesSelection libraryNode diff --git a/doc/diagrams/dependencies/CesiumIonClient.mmd b/doc/diagrams/dependencies/CesiumIonClient.mmd index 3eef3068a..64ee8335b 100644 --- a/doc/diagrams/dependencies/CesiumIonClient.mmd +++ b/doc/diagrams/dependencies/CesiumIonClient.mmd @@ -7,10 +7,11 @@ graph TD classDef dependencyNode fill:#fff,stroke:#ccc,color:#666,font-weight:bold,font-size:28px classDef libraryNode fill:#9f9,font-weight:bold,font-size:28px CesiumIonClient[CesiumIonClient] --> CesiumAsync[CesiumAsync] + CesiumIonClient[CesiumIonClient] --> CesiumGeospatial[CesiumGeospatial] CesiumIonClient[CesiumIonClient] --> CesiumUtility[CesiumUtility] CesiumIonClient[CesiumIonClient] --> OpenSSL_Crypto{{OpenSSL::Crypto}} CesiumIonClient[CesiumIonClient] --> httplib_httplib{{httplib::httplib}} CesiumIonClient[CesiumIonClient] --> modp_b64_modp_b64{{modp_b64::modp_b64}} CesiumIonClient[CesiumIonClient] --> picosha2_picosha2{{picosha2::picosha2}} class OpenSSL_Crypto,httplib_httplib,modp_b64_modp_b64,picosha2_picosha2 dependencyNode - class CesiumAsync,CesiumUtility,CesiumIonClient libraryNode + class CesiumAsync,CesiumGeospatial,CesiumUtility,CesiumIonClient libraryNode diff --git a/doc/diagrams/dependencies/CesiumUtility.mmd b/doc/diagrams/dependencies/CesiumUtility.mmd index 4b13dfd72..7d77b1b1e 100644 --- a/doc/diagrams/dependencies/CesiumUtility.mmd +++ b/doc/diagrams/dependencies/CesiumUtility.mmd @@ -6,9 +6,9 @@ title: CesiumUtility Dependency Graph graph TD classDef dependencyNode fill:#fff,stroke:#ccc,color:#666,font-weight:bold,font-size:28px classDef libraryNode fill:#9f9,font-weight:bold,font-size:28px + CesiumUtility[CesiumUtility] --> ada_ada{{ada::ada}} CesiumUtility[CesiumUtility] --> glm_glm{{glm::glm}} CesiumUtility[CesiumUtility] --> spdlog_spdlog{{spdlog::spdlog}} - CesiumUtility[CesiumUtility] --> uriparser_uriparser{{uriparser::uriparser}} CesiumUtility[CesiumUtility] --> zlib-ng_zlib-ng{{zlib-ng::zlib-ng}} - class glm_glm,spdlog_spdlog,uriparser_uriparser,zlib-ng_zlib-ng dependencyNode + class ada_ada,glm_glm,spdlog_spdlog,zlib-ng_zlib-ng dependencyNode class CesiumUtility libraryNode diff --git a/doc/diagrams/dependencies/all.mmd b/doc/diagrams/dependencies/all.mmd index 339f85764..c24fc8f01 100644 --- a/doc/diagrams/dependencies/all.mmd +++ b/doc/diagrams/dependencies/all.mmd @@ -7,9 +7,9 @@ config: graph TD classDef dependencyNode fill:#fff,stroke:#ccc,color:#666,font-weight:bold,font-size:28px classDef libraryNode fill:#9f9,font-weight:bold,font-size:28px + CesiumUtility[CesiumUtility] --> ada_ada{{ada::ada}} CesiumUtility[CesiumUtility] --> glm_glm{{glm::glm}} CesiumUtility[CesiumUtility] --> spdlog_spdlog{{spdlog::spdlog}} - CesiumUtility[CesiumUtility] --> uriparser_uriparser{{uriparser::uriparser}} CesiumUtility[CesiumUtility] --> zlib-ng_zlib-ng{{zlib-ng::zlib-ng}} Cesium3DTiles[Cesium3DTiles] --> CesiumUtility[CesiumUtility] Cesium3DTilesContent[Cesium3DTilesContent] --> Cesium3DTiles[Cesium3DTiles] @@ -70,7 +70,6 @@ graph TD Cesium3DTilesSelection[Cesium3DTilesSelection] --> spdlog_spdlog{{spdlog::spdlog}} Cesium3DTilesSelection[Cesium3DTilesSelection] --> spdlog_spdlog_header_only{{spdlog::spdlog_header_only}} Cesium3DTilesSelection[Cesium3DTilesSelection] --> tinyxml2_tinyxml2{{tinyxml2::tinyxml2}} - Cesium3DTilesSelection[Cesium3DTilesSelection] --> uriparser_uriparser{{uriparser::uriparser}} CesiumQuantizedMeshTerrain[CesiumQuantizedMeshTerrain] --> CesiumAsync[CesiumAsync] CesiumQuantizedMeshTerrain[CesiumQuantizedMeshTerrain] --> CesiumGeospatial[CesiumGeospatial] CesiumQuantizedMeshTerrain[CesiumQuantizedMeshTerrain] --> CesiumGltf[CesiumGltf] @@ -94,28 +93,29 @@ graph TD CesiumGltfWriter[CesiumGltfWriter] --> CesiumJsonWriter[CesiumJsonWriter] CesiumGltfWriter[CesiumGltfWriter] --> modp_b64_modp_b64{{modp_b64::modp_b64}} CesiumIonClient[CesiumIonClient] --> CesiumAsync[CesiumAsync] + CesiumIonClient[CesiumIonClient] --> CesiumGeospatial[CesiumGeospatial] CesiumIonClient[CesiumIonClient] --> CesiumUtility[CesiumUtility] CesiumIonClient[CesiumIonClient] --> OpenSSL_Crypto{{OpenSSL::Crypto}} CesiumIonClient[CesiumIonClient] --> httplib_httplib{{httplib::httplib}} CesiumIonClient[CesiumIonClient] --> modp_b64_modp_b64{{modp_b64::modp_b64}} CesiumIonClient[CesiumIonClient] --> picosha2_picosha2{{picosha2::picosha2}} - class glm_glm,spdlog_spdlog,uriparser_uriparser,zlib-ng_zlib-ng,libmorton_libmorton,Async_,spdlog_spdlog_header_only,unofficial_sqlite3_sqlite3,earcut,s2_s2,KTX_ktx,WebP_webp,WebP_webpdecoder,draco_draco,libjpeg-turbo_turbojpeg-static,meshoptimizer_meshoptimizer,modp_b64_modp_b64,nonstd_expected-lite,tinyxml2_tinyxml2,OpenSSL_Crypto,httplib_httplib,picosha2_picosha2 dependencyNode + class ada_ada,glm_glm,spdlog_spdlog,zlib-ng_zlib-ng,libmorton_libmorton,Async_,spdlog_spdlog_header_only,unofficial_sqlite3_sqlite3,earcut,s2_s2,KTX_ktx,WebP_webp,WebP_webpdecoder,draco_draco,libjpeg-turbo_turbojpeg-static,meshoptimizer_meshoptimizer,modp_b64_modp_b64,nonstd_expected-lite,tinyxml2_tinyxml2,OpenSSL_Crypto,httplib_httplib,picosha2_picosha2 dependencyNode class CesiumUtility,Cesium3DTiles,Cesium3DTilesReader,CesiumAsync,CesiumGeometry,CesiumGeospatial,CesiumGltf,CesiumGltfContent,CesiumGltfReader,Cesium3DTilesContent,CesiumJsonReader,CesiumQuantizedMeshTerrain,CesiumRasterOverlays,Cesium3DTilesSelection,CesiumJsonWriter,Cesium3DTilesWriter,CesiumGltfWriter,CesiumIonClient libraryNode linkStyle 0 stroke:#ff0029,stroke-width:8px - linkStyle 1,20,60 stroke:#377eb8,stroke-width:8px - linkStyle 2,63 stroke:#66a61e,stroke-width:8px + linkStyle 1 stroke:#377eb8,stroke-width:8px + linkStyle 2,20,60 stroke:#66a61e,stroke-width:8px linkStyle 3 stroke:#984ea3,stroke-width:8px - linkStyle 4,13,19,23,24,26,29,35,56,70,71,78,87 stroke:#00d2d5,stroke-width:8px - linkStyle 5,15,46,81 stroke:#ff7f00,stroke-width:8px + linkStyle 4,13,19,23,24,26,29,35,56,69,70,77,87 stroke:#00d2d5,stroke-width:8px + linkStyle 5,15,46,80 stroke:#ff7f00,stroke-width:8px linkStyle 6,48 stroke:#af8d00,stroke-width:8px - linkStyle 7,16,30,36,49,64,72,86 stroke:#7f80cd,stroke-width:8px - linkStyle 8,25,31,50,73 stroke:#b3e900,stroke-width:8px - linkStyle 9,32,51,65,74 stroke:#c42e60,stroke-width:8px - linkStyle 10,33,37,52,66,75,83 stroke:#a65628,stroke-width:8px - linkStyle 11,67,76 stroke:#f781bf,stroke-width:8px - linkStyle 12,34,53,77 stroke:#8dd3c7,stroke-width:8px + linkStyle 7,16,30,36,49,63,71,85 stroke:#7f80cd,stroke-width:8px + linkStyle 8,25,31,50,72 stroke:#b3e900,stroke-width:8px + linkStyle 9,32,51,64,73,86 stroke:#c42e60,stroke-width:8px + linkStyle 10,33,37,52,65,74,82 stroke:#a65628,stroke-width:8px + linkStyle 11,66,75 stroke:#f781bf,stroke-width:8px + linkStyle 12,34,53,76 stroke:#8dd3c7,stroke-width:8px linkStyle 14,58 stroke:#bebada,stroke-width:8px - linkStyle 17,38,68 stroke:#fb8072,stroke-width:8px + linkStyle 17,38,67 stroke:#fb8072,stroke-width:8px linkStyle 18 stroke:#80b1d3,stroke-width:8px linkStyle 21,61 stroke:#fdb462,stroke-width:8px linkStyle 22 stroke:#fccde5,stroke-width:8px @@ -127,13 +127,13 @@ graph TD linkStyle 42,57 stroke:#d95f02,stroke-width:8px linkStyle 43 stroke:#e7298a,stroke-width:8px linkStyle 44 stroke:#e6ab02,stroke-width:8px - linkStyle 45,85,90 stroke:#a6761d,stroke-width:8px + linkStyle 45,84,90 stroke:#a6761d,stroke-width:8px linkStyle 47 stroke:#0097ff,stroke-width:8px linkStyle 54 stroke:#00d067,stroke-width:8px linkStyle 55 stroke:#000000,stroke-width:8px - linkStyle 59,79 stroke:#252525,stroke-width:8px - linkStyle 62,80 stroke:#525252,stroke-width:8px - linkStyle 69,82,84 stroke:#737373,stroke-width:8px + linkStyle 59,78 stroke:#252525,stroke-width:8px + linkStyle 62,79 stroke:#525252,stroke-width:8px + linkStyle 68,81,83 stroke:#737373,stroke-width:8px linkStyle 88 stroke:#969696,stroke-width:8px linkStyle 89 stroke:#bdbdbd,stroke-width:8px linkStyle 91 stroke:#f43600,stroke-width:8px diff --git a/doc/img/dependency-graph.svg b/doc/img/dependency-graph.svg index b34ba7aa5..18fe90b27 100644 --- a/doc/img/dependency-graph.svg +++ b/doc/img/dependency-graph.svg @@ -1 +1 @@ -

CesiumUtility

glm::glm

spdlog::spdlog

uriparser::uriparser

zlib-ng::zlib-ng

Cesium3DTiles

Cesium3DTilesContent

Cesium3DTilesReader

CesiumAsync

CesiumGeometry

CesiumGeospatial

CesiumGltf

CesiumGltfContent

CesiumGltfReader

libmorton::libmorton

CesiumJsonReader

Async++

spdlog::spdlog_header_only

unofficial::sqlite3::sqlite3

earcut

s2::s2

KTX::ktx

WebP::webp

WebP::webpdecoder

draco::draco

libjpeg-turbo::turbojpeg-static

meshoptimizer::meshoptimizer

modp_b64::modp_b64

Cesium3DTilesSelection

CesiumQuantizedMeshTerrain

CesiumRasterOverlays

nonstd::expected-lite

tinyxml2::tinyxml2

CesiumJsonWriter

Cesium3DTilesWriter

CesiumGltfWriter

CesiumIonClient

OpenSSL::Crypto

httplib::httplib

picosha2::picosha2

 \ No newline at end of file +

CesiumUtility

ada::ada

glm::glm

spdlog::spdlog

zlib-ng::zlib-ng

Cesium3DTiles

Cesium3DTilesContent

Cesium3DTilesReader

CesiumAsync

CesiumGeometry

CesiumGeospatial

CesiumGltf

CesiumGltfContent

CesiumGltfReader

libmorton::libmorton

CesiumJsonReader

Async++

spdlog::spdlog_header_only

unofficial::sqlite3::sqlite3

earcut

s2::s2

KTX::ktx

WebP::webp

WebP::webpdecoder

draco::draco

libjpeg-turbo::turbojpeg-static

meshoptimizer::meshoptimizer

modp_b64::modp_b64

Cesium3DTilesSelection

CesiumQuantizedMeshTerrain

CesiumRasterOverlays

nonstd::expected-lite

tinyxml2::tinyxml2

CesiumJsonWriter

Cesium3DTilesWriter

CesiumGltfWriter

CesiumIonClient

OpenSSL::Crypto

httplib::httplib

picosha2::picosha2

 \ No newline at end of file From c3a18c608c5e5adabb78f67d38f0a6b5c2d24eb0 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Tue, 28 Jan 2025 12:53:57 -0500 Subject: [PATCH 21/28] Format --- CesiumIonClient/src/Connection.cpp | 2 +- CesiumUtility/include/CesiumUtility/Uri.h | 14 +++++++++----- CesiumUtility/src/Uri.cpp | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CesiumIonClient/src/Connection.cpp b/CesiumIonClient/src/Connection.cpp index 96f9ef313..f1a8fddce 100644 --- a/CesiumIonClient/src/Connection.cpp +++ b/CesiumIonClient/src/Connection.cpp @@ -494,7 +494,7 @@ Asset jsonToAsset(const rapidjson::Value& item) { std::optional generateApiUrl(const std::string& ionUrl) { Uri parsedIonUrl(ionUrl); - if(!parsedIonUrl) { + if (!parsedIonUrl) { return std::nullopt; } diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 481ce637e..720451a50 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -85,17 +85,21 @@ class Uri final { void setQueryValue(const std::string& key, const std::string& value); /** - * @brief Gets the scheme portion of the URI. If the URI was created without a scheme, this will return an empty string. + * @brief Gets the scheme portion of the URI. If the URI was created without a + * scheme, this will return an empty string. * - * @returns The scheme, or an empty string if the URI could not be parsed or has no scheme. + * @returns The scheme, or an empty string if the URI could not be parsed or + * has no scheme. */ std::string_view getScheme() const; /** - * @brief Gets the host portion of the URI. If the URI also specifies a non-default port, it will be included in the returned host. - * If the URI contains no host, this will return an empty string. + * @brief Gets the host portion of the URI. If the URI also specifies a + * non-default port, it will be included in the returned host. If the URI + * contains no host, this will return an empty string. * - * @returns The host, or an empty string if the URI could not be parsed or has no host. + * @returns The host, or an empty string if the URI could not be parsed or has + * no host. */ std::string_view getHost() const; diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 2a903f2ec..821bea19e 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -138,7 +138,7 @@ void Uri::setQueryValue(const std::string& key, const std::string& value) { } std::string_view Uri::getScheme() const { - if(!this->isValid()) { + if (!this->isValid()) { return {}; } @@ -146,7 +146,7 @@ std::string_view Uri::getScheme() const { } std::string_view Uri::getHost() const { - if(!this->isValid()) { + if (!this->isValid()) { return {}; } From db5868a18b94c8341636e5d0881b01e4afa9e952 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Wed, 29 Jan 2025 11:19:07 -0500 Subject: [PATCH 22/28] Address review comments --- CesiumUtility/include/CesiumUtility/Uri.h | 11 +++-------- CesiumUtility/src/Uri.cpp | 12 +----------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 720451a50..a323fd20f 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -29,8 +29,8 @@ class Uri final { Uri(const std::string& uri); /** * @brief Attempts to create a new Uri from a base URI and a relative URI. If - * the base URI is invalid or the relative URI string fails to parse, \ref - * isValid will return false. + * the base URI is invalid, only the relative URI string will be used. If the + * relative URI fails to parse, \ref isValid will return false. * * @param base The base URI that the relative URI is relative to. * @param relative A string containing a relative URI to attempt to parse. @@ -40,11 +40,6 @@ class Uri final { */ Uri(const Uri& base, const std::string& relative, bool useBaseQuery = false); - /** - * @brief Copy constructor for Uri. - */ - Uri(const Uri& uri); - /** * @brief Returns a string representation of the entire URI including path and * query parameters. @@ -72,7 +67,7 @@ class Uri final { * @returns The value of the given key in the query string, or `std::nullopt` * if not found. */ - const std::optional getQueryValue(const std::string& key); + std::optional getQueryValue(const std::string& key); /** * @brief Sets the given key in the URI's query parameters to the given value. diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 821bea19e..d677d9134 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -67,16 +67,7 @@ Uri::Uri(const std::string& uri) { if (result) { this->_url.emplace(std::move(result.value())); - this->_params.emplace(this->_url->get_search()); - } -} - -Uri::Uri(const Uri& uri) { - if (uri._url) { - this->_url.emplace(*uri._url); - this->_params.emplace(this->_url->get_search()); } - this->_hasScheme = uri._hasScheme; } Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { @@ -118,8 +109,7 @@ std::string Uri::toString() const { bool Uri::isValid() const { return this->_url && this->_params; } -const std::optional -Uri::getQueryValue(const std::string& key) { +std::optional Uri::getQueryValue(const std::string& key) { if (!this->isValid()) { return std::nullopt; } From 46ad1ac7472dc157bfab108e71823ff2b222ae8d Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Wed, 29 Jan 2025 17:16:17 -0500 Subject: [PATCH 23/28] Separate query handling from Uri --- .../src/LayerJsonTerrainLoader.cpp | 4 +- CesiumUtility/include/CesiumUtility/Uri.h | 99 ++++++++++++++----- CesiumUtility/src/Uri.cpp | 52 +++++----- 3 files changed, 105 insertions(+), 50 deletions(-) diff --git a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp index 1eb068083..79e8a719f 100644 --- a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp @@ -693,7 +693,9 @@ std::string resolveTileUrl( })); if (!layer.extensionsToRequest.empty()) { - uri.setQueryValue("extensions", layer.extensionsToRequest); + UriQueryParams params(uri); + params.setValue("extensions", layer.extensionsToRequest); + uri.setQuery(params.toQueryString()); } return uri.toString(); diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index a323fd20f..3113376ed 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -56,29 +56,6 @@ class Uri final { */ operator bool() const { return this->isValid(); } - /** - * @brief Obtains the value of the given key from the query string of the URI, - * if possible. - * - * If the URI can't be parsed, or the key doesn't exist in the - * query string, `std::nullopt` will be returned. - * - * @param key The key whose value will be obtained from the URI, if possible. - * @returns The value of the given key in the query string, or `std::nullopt` - * if not found. - */ - std::optional getQueryValue(const std::string& key); - - /** - * @brief Sets the given key in the URI's query parameters to the given value. - * If the key doesn't exist already, it will be added to the query parameters. - * Otherwise, the previous value will be overwritten. - * - * @param key The key to be added to the query string. - * @param value The value to be added to the query string. - */ - void setQueryValue(const std::string& key, const std::string& value); - /** * @brief Gets the scheme portion of the URI. If the URI was created without a * scheme, this will return an empty string. @@ -106,6 +83,13 @@ class Uri final { */ std::string_view getPath() const; + /** + * @brief Gets the query portion of the URI. + * + * @return The path, or empty string if the URI could not be parsed. + */ + std::string_view getQuery() const; + /** * @brief Sets the path portion of a URI to a new value. The other portions of * the URI are left unmodified, including any query parameters. @@ -114,6 +98,14 @@ class Uri final { */ void setPath(const std::string_view& path); + /** + * @brief Sets the query portion of a URI to a new value. The other portions + * of the URI are left unmodified. + * + * @param queryString The new query portion of the URI. + */ + void setQuery(const std::string_view& queryString); + /** * @brief Attempts to resolve a relative URI using a base URI. * @@ -326,7 +318,66 @@ class Uri final { private: std::optional _url = std::nullopt; - std::optional _params = std::nullopt; bool _hasScheme = false; }; + +/** + * @brief A class for parsing and manipulating the query string of a URI. + */ +class UriQueryParams final { +public: + UriQueryParams(const std::string_view& queryString) : _params(queryString) {} + UriQueryParams(const Uri& uri) : _params(uri.getQuery()) {} + + /** + * @brief Obtains the value of the given key from the query parameters, + * if possible. + * + * If the URI can't be parsed, or the key doesn't exist in the + * query string, `std::nullopt` will be returned. + * + * @param key The key whose value will be obtained from the query string, if + * possible. + * @returns The value of the given key in the query string, or `std::nullopt` + * if not found. + */ + std::optional getValue(const std::string& key) { + return this->_params.get(key); + } + + /** + * @brief Sets the given key in the query parameters to the given value. + * If the key doesn't exist already, it will be added to the query parameters. + * Otherwise, the previous value will be overwritten. + * + * @param key The key to be added to the query string. + * @param value The value to be added to the query string. + */ + void setValue(const std::string& key, const std::string& value) { + this->_params.set(key, value); + } + + /** + * @brief Returns true if this query string contains a value for the given + * key, or false otherwise. + * + * @param key The key to check. + */ + bool hasValue(const std::string& key) { return this->_params.has(key); } + + /** + * @brief Converts this object back into a query string, including all + * modifications that have been made. + */ + std::string toQueryString() const { return this->_params.to_string(); } + + inline auto begin() const { return this->_params.begin(); } + inline auto end() const { return this->_params.end(); } + inline auto front() const { return this->_params.front(); } + inline auto back() const { return this->_params.back(); } + +private: + ada::url_search_params _params; +}; + } // namespace CesiumUtility diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index d677d9134..6d55a7612 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -83,16 +83,17 @@ Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { if (result) { this->_url.emplace(std::move(result.value())); - this->_params.emplace(this->_url->get_search()); if (useBaseQuery) { + UriQueryParams baseParams(base); + UriQueryParams relativeParams(*this); // Set from relative to base to give priority to relative URL query string - for (const auto& [key, value] : *base._params) { - if (!this->_params->has(key)) { - this->_params->set(key, value); + for (const auto& [key, value] : baseParams) { + if (!relativeParams.hasValue(key)) { + relativeParams.setValue(key, value); } } - this->_url->set_search(this->_params->to_string()); + this->_url->set_search(relativeParams.toQueryString()); } } } @@ -107,40 +108,30 @@ std::string Uri::toString() const { : std::string(result.substr(FILE_PREFIX.length())); } -bool Uri::isValid() const { return this->_url && this->_params; } +bool Uri::isValid() const { return this->_url.has_value(); } -std::optional Uri::getQueryValue(const std::string& key) { - if (!this->isValid()) { - return std::nullopt; - } - - return this->_params->get(key); -} - -void Uri::setQueryValue(const std::string& key, const std::string& value) { +std::string_view Uri::getScheme() const { if (!this->isValid()) { - return; + return {}; } - this->_params->set(key, value); - // Update URL with modified params - this->_url->set_search(this->_params->to_string()); + return this->_hasScheme ? this->_url->get_protocol() : std::string_view{}; } -std::string_view Uri::getScheme() const { +std::string_view Uri::getHost() const { if (!this->isValid()) { return {}; } - return this->_hasScheme ? this->_url->get_protocol() : std::string_view{}; + return this->_url->get_host(); } -std::string_view Uri::getHost() const { +std::string_view Uri::getQuery() const { if (!this->isValid()) { return {}; } - return this->_url->get_host(); + return this->_url->get_search(); } std::string_view Uri::getPath() const { @@ -156,6 +147,10 @@ void Uri::setPath(const std::string_view& path) { this->_url->set_pathname(path); } +void Uri::setQuery(const std::string_view& queryString) { + this->_url->set_search(queryString); +} + std::string Uri::resolve( const std::string& base, const std::string& relative, @@ -173,12 +168,19 @@ std::string Uri::addQuery( return uri; } - parsedUri.setQueryValue(key, value); + UriQueryParams params(parsedUri); + params.setValue(key, value); + parsedUri.setQuery(params.toQueryString()); return parsedUri.toString(); } std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { - return std::string(Uri(uri).getQueryValue(key).value_or("")); + Uri parsedUri(uri); + if (!parsedUri.isValid()) { + return {}; + } + + return std::string(UriQueryParams(parsedUri).getValue(key).value_or("")); } // NOLINTEND(bugprone-unchecked-optional-access) From cea035d731c75806a039d8d6032ae53fc2af8c08 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 30 Jan 2025 10:04:09 -0500 Subject: [PATCH 24/28] Fix clang-tidy header warning --- CesiumUtility/include/CesiumUtility/Uri.h | 1 + CesiumUtility/src/Uri.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 3113376ed..0e97fd20f 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 6d55a7612..6cc29559f 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include From b0a16d2a4cef8d3880c58cd63f7fc00c10a73b64 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 30 Jan 2025 10:56:18 -0500 Subject: [PATCH 25/28] Fix docs --- CesiumUtility/include/CesiumUtility/Uri.h | 38 ++++++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 0e97fd20f..2bd18e9f5 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -139,8 +139,13 @@ class Uri final { * @param value The value to be added to the query string. * @returns The modified URI including the new query string parameter. * - * @deprecated Create a \ref Uri instance and use \ref Uri::setQueryValue - * instead. + * @deprecated Use the \ref UriQueryParams class: + * ``` + * Uri parsedUri(uri); + * UriQueryParams params(parsedUri); + * params.setValue(key, value); + * parsedUri.setQuery(params.toQueryString()); + * ``` */ static std::string addQuery( const std::string& uri, @@ -158,8 +163,12 @@ class Uri final { * @returns The value of the given key in the query string, or an empty string * if not found. * - * @deprecated Create a \ref Uri instance andthe member function \ref - * getQueryValue instead. + * @deprecated Use the \ref UriQueryParams class: + * ``` + * Uri parsedUri(uri); + * UriQueryParams params(parsedUri); + * params.getValue(key); + * ``` */ static std::string getQueryValue(const std::string& uri, const std::string& key); @@ -327,7 +336,22 @@ class Uri final { */ class UriQueryParams final { public: + /** + * @brief Creates a \ref UriQueryParams object from a query string. + * + * This query string should be in the format `key1=value1&key2=value2&key3=value3...`. + * This is the format returned by \ref Uri::getQuery. This string can include percent-encoded values. + * + * @param queryString The query string to parse into a query params object. + */ UriQueryParams(const std::string_view& queryString) : _params(queryString) {} + /** + * @brief Creates a \ref UriQueryParams object from a \ref Uri instance. + * + * This is equivalent to `UriQueryParams(uri.getQuery())`. + * + * @param uri The URI instance to obtain the query params from. + */ UriQueryParams(const Uri& uri) : _params(uri.getQuery()) {} /** @@ -368,13 +392,17 @@ class UriQueryParams final { /** * @brief Converts this object back into a query string, including all - * modifications that have been made. + * modifications that have been made. This result can be passed directly to \ref Uri::setQuery. */ std::string toQueryString() const { return this->_params.to_string(); } + /** @brief Returns an iterator pointing to the beginning of the query parameters. */ inline auto begin() const { return this->_params.begin(); } + /** @brief Returns an iterator pointing to the end of the query parameters. */ inline auto end() const { return this->_params.end(); } + /** @brief Returns the first element in the query parameters. */ inline auto front() const { return this->_params.front(); } + /** @brief Returns the last element in the query parameters. */ inline auto back() const { return this->_params.back(); } private: From f34c8fe55e71323adb00f9c29297ffec86d076dd Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 30 Jan 2025 10:58:59 -0500 Subject: [PATCH 26/28] Format --- CesiumUtility/include/CesiumUtility/Uri.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index 2bd18e9f5..b9bb0644f 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -339,8 +339,9 @@ class UriQueryParams final { /** * @brief Creates a \ref UriQueryParams object from a query string. * - * This query string should be in the format `key1=value1&key2=value2&key3=value3...`. - * This is the format returned by \ref Uri::getQuery. This string can include percent-encoded values. + * This query string should be in the format + * `key1=value1&key2=value2&key3=value3...`. This is the format returned by + * \ref Uri::getQuery. This string can include percent-encoded values. * * @param queryString The query string to parse into a query params object. */ @@ -348,7 +349,7 @@ class UriQueryParams final { /** * @brief Creates a \ref UriQueryParams object from a \ref Uri instance. * - * This is equivalent to `UriQueryParams(uri.getQuery())`. + * This is equivalent to `UriQueryParams(uri.getQuery())`. * * @param uri The URI instance to obtain the query params from. */ @@ -392,11 +393,13 @@ class UriQueryParams final { /** * @brief Converts this object back into a query string, including all - * modifications that have been made. This result can be passed directly to \ref Uri::setQuery. + * modifications that have been made. This result can be passed directly to + * \ref Uri::setQuery. */ std::string toQueryString() const { return this->_params.to_string(); } - /** @brief Returns an iterator pointing to the beginning of the query parameters. */ + /** @brief Returns an iterator pointing to the beginning of the query + * parameters. */ inline auto begin() const { return this->_params.begin(); } /** @brief Returns an iterator pointing to the end of the query parameters. */ inline auto end() const { return this->_params.end(); } From eb016f0c0ef65e0056d8f0c45998fae3c7618034 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Fri, 31 Jan 2025 13:25:07 -0500 Subject: [PATCH 27/28] Rename UriQueryParams -> UriQuery --- .../src/LayerJsonTerrainLoader.cpp | 2 +- CesiumUtility/include/CesiumUtility/Uri.h | 20 +++++++++---------- CesiumUtility/src/Uri.cpp | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp index 79e8a719f..6a947acb4 100644 --- a/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp +++ b/Cesium3DTilesSelection/src/LayerJsonTerrainLoader.cpp @@ -693,7 +693,7 @@ std::string resolveTileUrl( })); if (!layer.extensionsToRequest.empty()) { - UriQueryParams params(uri); + UriQuery params(uri); params.setValue("extensions", layer.extensionsToRequest); uri.setQuery(params.toQueryString()); } diff --git a/CesiumUtility/include/CesiumUtility/Uri.h b/CesiumUtility/include/CesiumUtility/Uri.h index b9bb0644f..a57f08116 100644 --- a/CesiumUtility/include/CesiumUtility/Uri.h +++ b/CesiumUtility/include/CesiumUtility/Uri.h @@ -139,10 +139,10 @@ class Uri final { * @param value The value to be added to the query string. * @returns The modified URI including the new query string parameter. * - * @deprecated Use the \ref UriQueryParams class: + * @deprecated Use the \ref UriQuery class: * ``` * Uri parsedUri(uri); - * UriQueryParams params(parsedUri); + * UriQuery params(parsedUri); * params.setValue(key, value); * parsedUri.setQuery(params.toQueryString()); * ``` @@ -163,10 +163,10 @@ class Uri final { * @returns The value of the given key in the query string, or an empty string * if not found. * - * @deprecated Use the \ref UriQueryParams class: + * @deprecated Use the \ref UriQuery class: * ``` * Uri parsedUri(uri); - * UriQueryParams params(parsedUri); + * UriQuery params(parsedUri); * params.getValue(key); * ``` */ @@ -334,10 +334,10 @@ class Uri final { /** * @brief A class for parsing and manipulating the query string of a URI. */ -class UriQueryParams final { +class UriQuery final { public: /** - * @brief Creates a \ref UriQueryParams object from a query string. + * @brief Creates a \ref UriQuery object from a query string. * * This query string should be in the format * `key1=value1&key2=value2&key3=value3...`. This is the format returned by @@ -345,15 +345,15 @@ class UriQueryParams final { * * @param queryString The query string to parse into a query params object. */ - UriQueryParams(const std::string_view& queryString) : _params(queryString) {} + UriQuery(const std::string_view& queryString) : _params(queryString) {} /** - * @brief Creates a \ref UriQueryParams object from a \ref Uri instance. + * @brief Creates a \ref UriQuery object from a \ref Uri instance. * - * This is equivalent to `UriQueryParams(uri.getQuery())`. + * This is equivalent to `UriQuery(uri.getQuery())`. * * @param uri The URI instance to obtain the query params from. */ - UriQueryParams(const Uri& uri) : _params(uri.getQuery()) {} + UriQuery(const Uri& uri) : _params(uri.getQuery()) {} /** * @brief Obtains the value of the given key from the query parameters, diff --git a/CesiumUtility/src/Uri.cpp b/CesiumUtility/src/Uri.cpp index 6cc29559f..e3a7d1aa3 100644 --- a/CesiumUtility/src/Uri.cpp +++ b/CesiumUtility/src/Uri.cpp @@ -84,8 +84,8 @@ Uri::Uri(const Uri& base, const std::string& relative, bool useBaseQuery) { this->_url.emplace(std::move(result.value())); if (useBaseQuery) { - UriQueryParams baseParams(base); - UriQueryParams relativeParams(*this); + UriQuery baseParams(base); + UriQuery relativeParams(*this); // Set from relative to base to give priority to relative URL query string for (const auto& [key, value] : baseParams) { if (!relativeParams.hasValue(key)) { @@ -167,7 +167,7 @@ std::string Uri::addQuery( return uri; } - UriQueryParams params(parsedUri); + UriQuery params(parsedUri); params.setValue(key, value); parsedUri.setQuery(params.toQueryString()); return parsedUri.toString(); @@ -179,7 +179,7 @@ std::string Uri::getQueryValue(const std::string& uri, const std::string& key) { return {}; } - return std::string(UriQueryParams(parsedUri).getValue(key).value_or("")); + return std::string(UriQuery(parsedUri).getValue(key).value_or("")); } // NOLINTEND(bugprone-unchecked-optional-access) From 8d319e438e8942d124e6d28eb14be6e3c4381363 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sat, 1 Feb 2025 21:35:28 +1100 Subject: [PATCH 28/28] Correct comment that is no longer true. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8d7adb9b1..8fabd58b7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,7 +46,7 @@ message(STATUS "VCPKG_OVERLAY_TRIPLETS ${VCPKG_OVERLAY_TRIPLETS}") # These packages are used in the public headers of Cesium libraries, so we need to distribute the headers and binaries # with the installation # Note that fmt is a public dependency of the vcpkg version of spdlog -# STB and ada-url aren't technically part of the public interface, but they're used by the downstream Cesium for Unreal project +# STB is not technically part of the public interface, but it is used by the downstream Cesium for Unreal project set(PACKAGES_PUBLIC asyncplusplus expected-lite fmt glm rapidjson spdlog stb ada-url) # These packages are used in the code and produce binaries, but are not part of the public interface. Therefore we need # to distribute the binaries for linking, but not the headers, as downstream consumers don't need them