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 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 bagder commented Jul 26, 2022

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

jay
jay approved these changes Jul 27, 2022
@bagder bagder closed this in f7e14fe Jul 27, 2022
@bagder bagder deleted the bagder/doh-url-blank branch Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmdline tool name lookup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants