From 54fe73f6800e62128e409825a0975645a33cfb5b Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:31:28 +0800 Subject: [PATCH 1/6] [BUG] Make file_http_server.h compile on Windows FileGetSuccess() takes the path as a reference to const and then called std::replace over filename.begin() and filename.end(). Those are const iterators, so std::replace cannot write through them and the _WIN32 branch does not compile. This is a plain C++ error, not an MSVC quirk: clang 18 and gcc 12 both reject it when that branch is instantiated. It went unnoticed because no translation unit in the repository includes this header, so CI never compiles it on any platform, but it is an installed header and a downstream Windows build hits it immediately. The separator substitution now runs on a local copy, and is included directly rather than arriving by way of socket_tools.h. Fixes #4299 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../opentelemetry/ext/http/server/file_http_server.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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..2b210d7e97 100644 --- a/ext/include/opentelemetry/ext/http/server/file_http_server.h +++ b/ext/include/opentelemetry/ext/http/server/file_http_server.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include #include @@ -52,10 +53,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(); From 9da83b29c7d288bc4739e7d08c3766a7b27cab69 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:32:39 +0800 Subject: [PATCH 2/6] Add CHANGELOG entry for #4300 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8baba4503c..863e5a634d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ Increment the: namespaces [#4303](https://github.com/open-telemetry/opentelemetry-cpp/pull/4303) +* [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) From 577ef811714ffd938447e440bfb82fba1bbd4dea Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:54:43 +0800 Subject: [PATCH 3/6] Compile file_http_server.h in CI so the build fix cannot regress Review of #4300 noted the fix had no regression protection: nothing in the repository includes this installed header, so every CI job could be green without the header ever being compiled, which is how the _WIN32 build error survived in the first place. Add a compile-only target (CMake OBJECT library and Bazel cc_library) whose sole translation unit includes the header. CI now compiles the POSIX branch on Linux and macOS and the _WIN32 branch on the Windows runners, so a future const-iterator std::replace, or any other build break in this header, fails immediately. Also add the direct include the header needs for std::ostringstream; like the include, it was resolving only because socket_tools.h happens to provide it, and the new compile target is exactly what would surface that. Fixes #4299 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../ext/http/server/file_http_server.h | 1 + ext/test/http/BUILD | 13 +++++++++++++ ext/test/http/CMakeLists.txt | 6 ++++++ ext/test/http/file_http_server_header_compile.cc | 9 +++++++++ 4 files changed, 29 insertions(+) create mode 100644 ext/test/http/file_http_server_header_compile.cc 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 2b210d7e97..5f585bcbc4 100644 --- a/ext/include/opentelemetry/ext/http/server/file_http_server.h +++ b/ext/include/opentelemetry/ext/http/server/file_http_server.h @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include 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..7014f00384 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -22,3 +22,9 @@ 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..9aa3d2c6cc --- /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" From 6d4e83daf7913ebac54a05c8169869b456b554be Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:19:33 +0800 Subject: [PATCH 4/6] Fix two performance-faster-string-find warnings the compile test exposed The new header-compile translation unit is the first thing in the build to include file_http_server.h, so clang-tidy now lints it and counted two pre-existing warnings against the ratchet: find() and find_last_of() called with a single-character string literal. Pass the character directly so the header is lint-clean under the existing limit rather than raising it. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- ext/include/opentelemetry/ext/http/server/file_http_server.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 5f585bcbc4..7974285acd 100644 --- a/ext/include/opentelemetry/ext/http/server/file_http_server.h +++ b/ext/include/opentelemetry/ext/http/server/file_http_server.h @@ -85,7 +85,7 @@ 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; }; @@ -104,7 +104,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; From 19749b698f88a0fa98a3977b6bd1be9a11f9dc0c Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:23:44 +0800 Subject: [PATCH 5/6] Fix extra-semicolon errors and IWYU on the header-compile probe The header-compile translation unit makes the maintainer -Wextra-semi -Werror configuration compile file_http_server.h for the first time, turning three pre-existing extra semicolons after in-class function bodies into errors. Drop them. The probe intentionally uses no symbol from the header, so mark its include IWYU pragma: keep. Also drop the unused and guard behind _WIN32, where its only use (std::replace) lives. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../opentelemetry/ext/http/server/file_http_server.h | 11 ++++++----- ext/test/http/file_http_server_header_compile.cc | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) 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 7974285acd..83707b6a60 100644 --- a/ext/include/opentelemetry/ext/http/server/file_http_server.h +++ b/ext/include/opentelemetry/ext/http/server/file_http_server.h @@ -3,13 +3,14 @@ #pragma once -#include #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" @@ -30,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. @@ -76,7 +77,7 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer return true; } return false; - }; + } /** * Returns the extension of a file @@ -88,7 +89,7 @@ class FileHttpServer : public HTTP_SERVER_NS::HttpServer 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 diff --git a/ext/test/http/file_http_server_header_compile.cc b/ext/test/http/file_http_server_header_compile.cc index 9aa3d2c6cc..6e5eff7d17 100644 --- a/ext/test/http/file_http_server_header_compile.cc +++ b/ext/test/http/file_http_server_header_compile.cc @@ -6,4 +6,4 @@ // _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" +#include "opentelemetry/ext/http/server/file_http_server.h" // IWYU pragma: keep From a9b17f90a3550353bebe862dd9dc7fed282366ab Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:43:28 +0800 Subject: [PATCH 6/6] Fix Format CI: cmake-format ext/test/http/CMakeLists.txt Apply cmake-format with the repo config to the file_http_server_header_compile block. --- ext/test/http/CMakeLists.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ext/test/http/CMakeLists.txt b/ext/test/http/CMakeLists.txt index 7014f00384..28102064e2 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -23,8 +23,10 @@ gtest_add_tests( 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) +# 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)