Skip to content

tool: fix --capath when proxy support is disabled#12089

Closed
hwti wants to merge 1 commit into
curl:masterfrom
hwti:fix-capath-without-proxy
Closed

tool: fix --capath when proxy support is disabled#12089
hwti wants to merge 1 commit into
curl:masterfrom
hwti:fix-capath-without-proxy

Conversation

@hwti

@hwti hwti commented Oct 11, 2023

Copy link
Copy Markdown
Contributor

After 95e8515, --capath always sets CURLOPT_PROXY_CAPATH, which fails with CURLE_UNKNOWN_OPTION when proxy support is disabled.

@bagder

bagder commented Oct 12, 2023

Copy link
Copy Markdown
Member

We try hard to not move CURL_DISABLE_ checks into the tool code as the tool and library should ideally remain as independent as possible. I think a better fix would be to let the tool handle the run-time error correctly, probably in the same way it already handles CURLE_NOT_BUILT_IN. What do you think about that?

@hwti

hwti commented Oct 12, 2023

Copy link
Copy Markdown
Contributor Author

So something like ?

          else if(result == CURLE_UNKNOWN_OPTION && !config->proxy_capath) {
            /* If proxy support is disabled, and we only have --capath, ignore the error */
          }

I wonder CURLE_UNKNOWN_OPTION is the correct error code.
It seems to be used for several proxy configs, since #ifndef CURL_DISABLE_PROXY flag many case CURLOPT_PROXY_*:.
But when SSL support is disabled, or the backend does not support CAPATH, setting CURLOPT_PROXY_CAPATH returns CURLE_NOT_BUILT_IN.

@bagder

bagder commented Oct 16, 2023

Copy link
Copy Markdown
Member

I was thinking like the patch below.

The *setopt() code returns CURLE_NOT_BUILT_IN for a lot of cases where it explicitly knows that the support was disabled from the build, but it also returns CURLE_UNKNOWN_OPTION for the cases when the entire option handling was removed. The latter return code will be returned for most/all CURLOPT_PROXY_* options when proxy support was disabled in the build. But also: if you would use an older libcurl without support for this option, before it was introduced, it would also return CURLE_UNKNOWN_OPTION for it.

So I think the safest and most sane approach is to check for both and treat them the same:

diff --git a/src/tool_operate.c b/src/tool_operate.c
index b60f57e0d..697b64e38 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1774,11 +1774,12 @@ static CURLcode single_transfer(struct GlobalConfig *global,
        if(config->proxy_capath || config->capath) {
          result = res_setopt_str(curl, CURLOPT_PROXY_CAPATH,
                                  (config->proxy_capath ?
                                   config->proxy_capath :
                                   config->capath));
-          if(result == CURLE_NOT_BUILT_IN) {
+          if((result == CURLE_NOT_BUILT_IN) ||
+             (result == CURLE_UNKNOWN_OPTION)) {
            if(config->proxy_capath) {
              warnf(global,
                    "ignoring --proxy-capath, not supported by libcurl");
            }
          }

After 95e8515, --capath always sets CURLOPT_PROXY_CAPATH, which fails with
CURLE_UNKNOWN_OPTION when proxy support is disabled.
@hwti

hwti commented Oct 16, 2023

Copy link
Copy Markdown
Contributor Author

OK, this mean that using --proxy-capath when proxy support is disabled would only be a warning.
But since --proxy is still error, it's fine (no risk to send a direct request when the user wanted to use a proxy).

@hwti hwti force-pushed the fix-capath-without-proxy branch from 2b6b958 to fef672d Compare October 16, 2023 12:30
@bagder bagder closed this in 014ce7c Oct 21, 2023
@bagder

bagder commented Oct 21, 2023

Copy link
Copy Markdown
Member

Thanks!

@hwti hwti deleted the fix-capath-without-proxy branch October 23, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants