diff --git a/CHANGELOG.md b/CHANGELOG.md index 819a741b73..8251d3d95f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ Increment the: * [CODE HEALTH] Move remaining API test helpers into anonymous namespaces [#4301](https://github.com/open-telemetry/opentelemetry-cpp/pull/4301) +* [BUG] Make file_http_server.h compile on Windows + [#4300](https://github.com/open-telemetry/opentelemetry-cpp/pull/4300) + * [CODE HEALTH] Move metrics storage test fixtures into anonymous namespace [#4286](https://github.com/open-telemetry/opentelemetry-cpp/pull/4286) diff --git a/ext/include/opentelemetry/ext/http/server/file_http_server.h b/ext/include/opentelemetry/ext/http/server/file_http_server.h index cb28d01fa3..83707b6a60 100644 --- a/ext/include/opentelemetry/ext/http/server/file_http_server.h +++ b/ext/include/opentelemetry/ext/http/server/file_http_server.h @@ -4,10 +4,13 @@ #pragma once #include -#include +#include #include #include #include +#ifdef _WIN32 +# include // std::replace, used only in the _WIN32 path below +#endif #include "opentelemetry/ext/http/server/http_server.h" @@ -28,7 +31,7 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer os << host << ":" << port; setServerName(os.str()); addListeningPort(port); - }; + } /** * Set the HTTP server to serve static files from the root of host:port. @@ -52,10 +55,15 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer bool FileGetSuccess(const std::string &filename, std::vector &result) { #ifdef _WIN32 - std::replace(filename.begin(), filename.end(), '/', '\\'); + // std::replace writes through its iterators, so it needs a modifiable string rather than + // the const parameter. + std::string path = filename; + std::replace(path.begin(), path.end(), '/', '\\'); +#else + const std::string &path = filename; #endif std::streampos size; - std::ifstream file(filename, std::ios::in | std::ios::binary | std::ios::ate); + std::ifstream file(path, std::ios::in | std::ios::binary | std::ios::ate); if (file.is_open()) { size = file.tellg(); @@ -69,7 +77,7 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer return true; } return false; - }; + } /** * Returns the extension of a file @@ -78,10 +86,10 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer */ std::string GetMimeContentType(const std::string &filename) { - std::string file_ext = filename.substr(filename.find_last_of(".") + 1); + std::string file_ext = filename.substr(filename.find_last_of('.') + 1); auto file_type = mime_types_.find(file_ext); return (file_type != mime_types_.end()) ? file_type->second : HTTP_SERVER_NS::CONTENT_TYPE_TEXT; - }; + } /** * Returns the standardized name of a file by removing backslashes, and @@ -97,7 +105,7 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer } // If filename appears to be a directory, serve the hypothetical index.html // file there - if (name.find(".") == std::string::npos) + if (name.find('.') == std::string::npos) name += "/index.html"; return name; diff --git a/ext/test/http/BUILD b/ext/test/http/BUILD index 2e6a07e688..5a2e96997f 100644 --- a/ext/test/http/BUILD +++ b/ext/test/http/BUILD @@ -1,8 +1,21 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 +load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_cc//cc:cc_test.bzl", "cc_test") +# file_http_server.h is an installed header no other target includes, so it was never compiled. +# This library compiles it (the POSIX branch here, the Windows branch on the Windows runners) so +# a build regression in it is caught. +cc_library( + name = "file_http_server_header_compile", + srcs = ["file_http_server_header_compile.cc"], + tags = ["test"], + deps = [ + "//ext:headers", + ], +) + cc_test( name = "curl_http_test", srcs = [ diff --git a/ext/test/http/CMakeLists.txt b/ext/test/http/CMakeLists.txt index b7707bd8c2..28102064e2 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -22,3 +22,11 @@ gtest_add_tests( TARGET ${URL_PARSER_FILENAME} TEST_PREFIX ext.http.urlparser. TEST_LIST ${URL_PARSER_FILENAME}) + +# file_http_server.h is an installed header that no other target includes, so it +# was never compiled. This object library compiles it (the POSIX branch here, +# the Windows branch on the Windows runners) so a build regression in it is +# caught. It is not run. +add_library(file_http_server_header_compile OBJECT + file_http_server_header_compile.cc) +target_link_libraries(file_http_server_header_compile PRIVATE opentelemetry_ext) diff --git a/ext/test/http/file_http_server_header_compile.cc b/ext/test/http/file_http_server_header_compile.cc new file mode 100644 index 0000000000..6e5eff7d17 --- /dev/null +++ b/ext/test/http/file_http_server_header_compile.cc @@ -0,0 +1,9 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +// file_http_server.h is an installed public header that no other translation unit in the +// repository includes, so nothing compiled it and a build error could sit there unnoticed (the +// _WIN32 branch did exactly that). This translation unit exists only to compile the header, so +// CI builds the POSIX branch here and the Windows branch on the Windows runners. It has no +// main() and is never run. +#include "opentelemetry/ext/http/server/file_http_server.h" // IWYU pragma: keep