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

output warning for debug-enabled version #10278

Closed
wants to merge 3 commits into from
Closed

Conversation

bagder
Copy link
Member

@bagder bagder commented Jan 11, 2023

A debug-enabled version of curl is not intended for production

A debug-enabled version of curl is not intended for production
@jay
Copy link
Member

jay commented Jan 11, 2023

Ref: https://curl.se/mail/lib-2023-01/0039.html

I think it's too much but it looks like the list disagrees. Do you have a reference to the issue that was reported with the unintentional use of debugbuild? What about put it in curl_version, all caps, DEBUGBUILD. That way it is also visible from libcurl. Nevermind that DEBUGBUILD can override the version string if CURL_VERSION env is set...

curl 7.87.1-DEV (i386-pc-win32) libcurl/7.87.1-DEV DEBUGBUILD OpenSSL/1.1.1s nghttp2/1.50.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS Debug HSTS HTTP2 HTTPS-proxy IPv6 Largefile NTLM SSL Unicode UnixSockets
diff --git a/lib/version.c b/lib/version.c
index a69e2ca..dae8335 100644
--- a/lib/version.c
+++ b/lib/version.c
@@ -109,7 +109,7 @@ static void zstd_version(char *buf, size_t bufsz)
  * zeros in the data.
  */
 
-#define VERSION_PARTS 16 /* number of substrings we can concatenate */
+#define VERSION_PARTS 17 /* number of substrings we can concatenate */
 
 char *curl_version(void)
 {
@@ -173,6 +173,9 @@ char *curl_version(void)
 #endif
 
   src[i++] = LIBCURL_NAME "/" LIBCURL_VERSION;
+#ifdef DEBUGBUILD
+  src[i++] = "DEBUGBUILD";
+#endif
 #ifdef USE_SSL
   Curl_ssl_version(ssl_version, sizeof(ssl_version));
   src[i++] = ssl_version;

@bagder
Copy link
Member Author

bagder commented Jan 11, 2023

The debug-by-mistake was in here #10269

I don't think putting DEBUGBUILD in there is much different to what we do already do today. It doesn't stress or emphasize how unwise it is to use such build in production.

@jay
Copy link
Member

jay commented Jan 11, 2023

It looks as though that reporter is using libcurl but not the curl tool. What about in multi,

> curld  -v https://curl.se -o NUL
* !!! WARNING !!!
* This is a debug build of libcurl, do not use in production.
* STATE: INIT => CONNECT handle 0x1d37ae8; line 1917 (connection #-5000)
* Added connection 0. The cache now contains 1 members
diff --git a/lib/multi.c b/lib/multi.c
index b6187cd..4032748 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1861,6 +1861,15 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
     multistate(data, MSTATE_COMPLETED);
   }
 
+#ifdef DEBUGBUILD
+  if(!multi->warned) {
+    infof(data, "!!! WARNING !!!");
+    infof(data, "This is a debug build of libcurl, "
+          "do not use in production.");
+    multi->warned = true;
+  }
+#endif
+
   do {
     /* A "stream" here is a logical stream if the protocol can handle that
        (HTTP/2), or the full connection for older protocols */
diff --git a/lib/multihandle.h b/lib/multihandle.h
index 7dd6a0a..6cda65d 100644
--- a/lib/multihandle.h
+++ b/lib/multihandle.h
@@ -170,6 +170,9 @@ struct Curl_multi {
 #endif
   BIT(dead); /* a callback returned error, everything needs to crash and
                 burn */
+#ifdef DEBUGBUILD
+  BIT(warned);                 /* true after user warned of DEBUGBUILD */
+#endif
 };
 
 #endif /* HEADER_CURL_MULTIHANDLE_H */

@bagder
Copy link
Member Author

bagder commented Jan 12, 2023

Right, maybe doing both would make sense. (as proposed by @dfandrich on the list)

@bagder bagder changed the title curl: output warning at --verbose output for debug-enabled version output warning for debug-enabled version Jan 12, 2023
@bagder
Copy link
Member Author

bagder commented Jan 12, 2023

I'll merge this, but let's keep our ears and eyes open to see if this causes too much problems.

@bagder bagder closed this in 7d3b167 Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants