Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix ECH to always enable HTTPS RR #15648

Closed
wants to merge 6 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 26, 2024

The ECH feature cannot be built without HTTPS RR.

ECH automatically implied HTTPS RR in ./configure but not in CMake,
winbuild, documentation.

Also update documentation and CI configs.

Follow-up to a362962 #11922

@vszakats vszakats added the build label Nov 26, 2024
@github-actions github-actions bot added the CI Continuous Integration label Nov 26, 2024
@vszakats
Copy link
Member Author

I'm casually considering this a bugfix.

@vszakats vszakats force-pushed the ech-to-enable-httpsrr branch from b2e3c19 to fe67bc8 Compare November 26, 2024 18:17
@vszakats vszakats changed the title build: fix ECH to always enable HTTPSRR build: fix ECH to always enable HTTPS RR Nov 26, 2024
@vszakats
Copy link
Member Author

Any objections for merging this in the bugfix cycle?

The essence is lib/curl_setup.h activates USE_HTTPSRR
now instead of ./configure. The rest is CI and docs updates.

--- a/configure.ac
+++ b/configure.ac
@@ -5022,8 +5022,6 @@ if test "x$want_ech" != "xno"; then
 
   dnl now deal with whatever we found
   if test "x$ECH_ENABLED" = "x1"; then
-    dnl force pre-requisites for ECH
-    AC_DEFINE(USE_HTTPSRR, 1, [force HTTPS RR support for ECH])
     AC_DEFINE(USE_ECH, 1, [if ECH support is available])
     AC_MSG_RESULT($ECH_SUPPORT)
     experimental="$experimental ECH"
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -290,6 +290,14 @@
 #  define CURL_DISABLE_HTTP_AUTH 1
 #endif
 
+/*
+ * ECH requires HTTPSRR.
+ */
+
+#if defined(USE_ECH) && !defined(USE_HTTPSRR)
+#  define USE_HTTPSRR
+#endif
+
 /* ================================================================ */
 /* No system header file shall be included in this file before this */
 /* point.                                                           */

@vszakats vszakats added the TLS label Nov 26, 2024
@vszakats vszakats closed this in 2f03242 Nov 28, 2024
@vszakats vszakats deleted the ech-to-enable-httpsrr branch November 28, 2024 10:56
vszakats added a commit to curl/curl-for-win that referenced this pull request Dec 11, 2024
Also drop a build setting that is no longer necessary after:

curl/curl@2f03242
curl/curl#15648
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CI Continuous Integration TLS
Development

Successfully merging this pull request may close these issues.

1 participant