Skip to content

[civetweb] download instead of bundle, bump from 1.15 to 1.16, and define proper CMake target#21947

Draft
ferdymercury wants to merge 49 commits intoroot-project:masterfrom
ferdymercury:bcivet
Draft

[civetweb] download instead of bundle, bump from 1.15 to 1.16, and define proper CMake target#21947
ferdymercury wants to merge 49 commits intoroot-project:masterfrom
ferdymercury:bcivet

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 17, 2026

Test Results

    21 files      21 suites   3d 13h 46m 7s ⏱️
 3 852 tests  3 850 ✅ 1 💤 1 ❌
73 248 runs  73 236 ✅ 9 💤 3 ❌

For more details on these failures, see this check.

Results for commit de6747d.

♻️ This comment has been updated with latest results.

@dpiparo dpiparo self-assigned this Apr 19, 2026
@ferdymercury ferdymercury marked this pull request as ready for review April 20, 2026 13:18
Comment thread net/http/CMakeLists.txt
@ferdymercury ferdymercury added the skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff. label Apr 21, 2026
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

@linev for Fedora we'd need root-project/root-ci-images#111

endif()
endif()
if (NOT builtin_civetweb)
target_compile_definitions(civetweb::civetweb INTERFACE _EXTERNAL_CIVETWEB) # temporary hack for: with external civetweb one gets failure R__memcompress
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

TO-DO investigate

Comment thread cmake/modules/SearchInstalledSoftware.cmake Outdated
Comment thread builtins/civetweb/CMakeLists.txt Outdated
@ferdymercury

This comment was marked as outdated.

Comment thread builtins/openssl/CMakeLists.txt Outdated
@ferdymercury ferdymercury added the clean build Ask CI to do non-incremental build on PR label Apr 22, 2026
@ferdymercury ferdymercury reopened this Apr 22, 2026
@ferdymercury ferdymercury removed the clean build Ask CI to do non-incremental build on PR label Apr 22, 2026
ferdymercury added a commit to ferdymercury/root that referenced this pull request Apr 23, 2026
Compilation with builtin_openssl probably just worked fine because it picked system-wide openssl headers rather than the builtin ones.

Found out while investigating failures in root-project#21947
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Finally! Compilation failures on Mac solved, now builds on all platforms.
I guess this PR should be preceded by #22026

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

@linev you mentioned that on some platforms the system-civetweb was not stable. Do you remember which platforms that was or how we can test the current status? Was it running a specific test? Do you remember exactly which ones?

I see opensuse having these flags:
https://code.opensuse.org/package/civetweb/blob/master/f/civetweb.spec


  | export CFLAGS="%optflags -DUSE_X_DOM_SOCKET"
-- | --
  |  
  | %cmake -DCIVETWEB_ENABLE_WEBSOCKETS=ON \
  | -DCIVETWEB_BUILD_TESTING=OFF \
  | -DCIVETWEB_ENABLE_CXX=ON \
  | -DCIVETWEB_ENABLE_SSL=ON \
  | -DCIVETWEB_SSL_OPENSSL_API_1_1=OFF \
  | -DCIVETWEB_SSL_OPENSSL_API_3_0=ON \
  | -DCIVETWEB_ENABLE_ZLIB=ON \
  | -DCIVETWEB_ENABLE_SSL_DYNAMIC_LOADING=OFF

and Debian here has some extra patches on 1.16

https://sources.debian.org/patches/civetweb/1.16+dfsg-4/

It looks like Debian 1.16 patches contain some fixes for DoS attacks and CVEs, so I think it would be beneficial if we switched to builtin_civetweb=OFF At least on the platforms where you are not aware of crashes.

@linev
Copy link
Copy Markdown
Member

linev commented Apr 27, 2026

This was my attempt to make OpenSUSE package better.
But even with all necessary flags sometime it works (within particular OS build) and sometime does not.

This is main problem for me - even I think all necessary flags are provided something is still wrong.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Thanks for the feedback.
How can I check eg if it's working stable on my local Ubuntu 22 if I build with builtin_civetweb=OFF ? And then running some tests? or just root --web=ON ?
If it works, then we could set builtin_civetweb OFF for Ubuntus and ON for opensuse ?

@linev
Copy link
Copy Markdown
Member

linev commented Apr 27, 2026

How can I check eg if it's working stable on my local Ubuntu 22

It crashed when used with RBrowser or with simple THttpServer examples.
I also had report from user about similar failures on other platforms.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

I also had report from user about similar failures on other platforms.

Ok, thanks. What can be worrying is that users downloading the release binaries are (most of them unknowingly) using a builtin civetweb, even if they installed libcivetweb-dev.

There are known vulnerabilities in civetweb. Once the maintainer releases version 1.17, users having installed the system pacakge would (soonish) automatically get security updates for
https://nvd.nist.gov/vuln/detail/CVE-2025-55763
https://nvd.nist.gov/vuln/detail/CVE-2026-5789
civetweb/civetweb#1353

but ROOT 6.40 downloaded from binaries would still be (unknowingly) vulnerable for maybe a long time.

I also had report from user about similar failures on other platforms.

It could be useful if we document in the code or in a GH issue which platforms and what civetweb versions they were using to keep track of when this potentially gets solved. That way, we can then say find_package(civetweb 1.17) else use builtin.

In my opinion, we should also add patches from CVE vulnerabilities to the builtin version as soon as possible on top of this PR.

dpiparo pushed a commit that referenced this pull request Apr 27, 2026
Compilation with builtin_openssl probably just worked fine because it picked system-wide openssl headers rather than the builtin ones.

Found out while investigating failures in #21947
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants