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

tool_operate: make --doh-url "" switch it off #9207

Closed
wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Jul 26, 2022

A possible future addition could be to parse the URL first too to verify
that it is valid before trying to use it.

@jay
Copy link
Member

jay commented Jul 26, 2022

I'm skeptical about doing this with strings because we may have logic elsewhere that needs to be addressed (as is the case here, see the squashme for env logic) and that can lead to bugs. An alternative would be handle it during parsing,

diff --git a/src/tool_getparam.c b/src/tool_getparam.c
index 9bbd51d..0aec45d 100644
--- a/src/tool_getparam.c
+++ b/src/tool_getparam.c
@@ -700,7 +700,10 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
           return err;
         break;
       case 'C': /* doh-url */
-        GetStr(&config->doh_url, nextarg);
+        if(nextarg && *nextarg)
+          GetStr(&config->doh_url, nextarg);
+        else /* this option is documented as unset by an empty string */
+          Curl_safefree(config->doh_url);
         break;
       case 'd': /* ciphers */
         GetStr(&config->cipher_list, nextarg);

A possible future addition could be to parse the URL first too to verify
that it is valid before trying to use it.

Assisted-by: Jay Satiro
Closes #9207
@bagder
Copy link
Member Author

bagder commented Jul 26, 2022

Yeah, setting it to NULL again is clearly a better approach.

@bagder bagder closed this in f7e14fe Jul 27, 2022
@bagder bagder deleted the bagder/doh-url-blank branch July 27, 2022 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmdline tool name lookup DNS and related tech
Development

Successfully merging this pull request may close these issues.

None yet

2 participants