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.

Sign up for GitHub

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

Jump to bottom

Wrong version mentioned for --disable (-q) in docs/options-in-versions #11710

Closed
pszlazak opened this issue Aug 22, 2023 · 6 comments
Closed

Wrong version mentioned for --disable (-q) in docs/options-in-versions #11710

pszlazak opened this issue Aug 22, 2023 · 6 comments
Labels
cmdline tool documentation

Comments

@pszlazak
Copy link
Contributor

I did this

Having following settings in ~/.curlrc:

--write-out \n
--netrc-optional

I executed:

curl --disable -v https://some.host/

In the output I noticed:

* Couldn't find host some.host in the .netrc file; using defaults

I expected the following

I assumed config options from ~/.curlrc will not be used, especially --netrc-optional.

According to docs/options-in-versions option to disable reading curlrc file was introduced in 5.0:

--disable (-q)                       5.0

This is a little bit confusing. I guess it could be like this instead:

--disable (-q)                       7.49.0 (5.0)

curl/libcurl version

curl 7.29.0 (x86_64-redhat-linux-gnu) libcurl/7.29.0 NSS/3.44 zlib/1.2.7 libidn/1.28 libssh2/1.8.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smtp smtps telnet tftp
Features: AsynchDNS GSS-Negotiate IDN IPv6 Largefile NTLM NTLM_WB SSL libz unix-sockets

operating system

Red Hat Enterprise Linux Server release 7.5 (Maipo)
@jay
Copy link
Member

jay commented Aug 22, 2023 •
edited

curl 5.0 has --disable. It has to be used as the first curl parameter. Is that the actual command line you executed? Maybe you are seeing a bug that is not present in later versions. What happens if you use -q as the first parameter?

edit: It looks like --disable existed as an option in earlier versions but was not implemented. Here is the code from 7.29.0:

curl/src/tool_operate.c

Lines 201 to 212 in bf633a5

if((argc > 1) &&
(!curlx_strnequal("--", argv[1], 2) && (argv[1][0] == '-')) &&
strchr(argv[1], 'q')) {
/*
* The first flag, that is not a verbose name, but a shortname
* and it includes the 'q' flag!
*/
;
}
else {
parseconfig(NULL, config); /* ignore possible failure */
}

Later it was added in e200034 (precedes 7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0)

jay added a commit to jay/curl that referenced this issue Aug 22, 2023
@jay
disable.d: explain --disable not implemented prior to 7.50.0
b28d9e9 
Option -q/--disable was added in 5.0 but only -q was actually
implemented. Later --disable was implemented in e200034 (precedes
7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0).

Reported-by: pszlazak@users.noreply.github.com

Fixes curl#11710
Closes #xxxx
jay added a commit to jay/curl that referenced this issue Aug 22, 2023
@jay
disable.d: explain --disable not implemented prior to 7.50.0
f7b854d 
Option -q/--disable was added in 5.0 but only -q was actually
implemented. Later --disable was implemented in e200034 (precedes
7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0).

Reported-by: pszlazak@users.noreply.github.com

Fixes curl#11710
Closes #xxxx
@jay jay mentioned this issue Aug 22, 2023
Closed
jay added a commit to jay/curl that referenced this issue Aug 22, 2023
@jay
disable.d: explain --disable not implemented prior to 7.50.0
5939a44 
Option -q/--disable was added in 5.0 but only -q was actually
implemented. Later --disable was implemented in e200034 (precedes
7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0).

Reported-by: pszlazak@users.noreply.github.com

Fixes curl#11710
Closes #xxxx
@pszlazak
Copy link
Contributor Author

I noticed that --disable was added as alias for -q in 7.49.0:
curl-7_48_0...curl-7_49_0 (click 'Files changed' tab and open diff for scr/tool_operate.c)

But I missed that there was one more correction in 7.50.0. It was even mentioned in changelog as:

curl: fix -q [regression]

My main suggestion is to clearly state this in docs/options-in-versions:

--disable (-q)                       7.50.0 (5.0)

But that would be only option having appearance in two different versions...

BTW - yes, I was testing this as the first curl parameter.

@bagder
Copy link
Member

bagder commented Aug 22, 2023 •
edited

Adding that info to the options-in-versions file would require test 971 to get fixed (I never considered the case where it might need two versions for the same option). I think getting the details correct in disable.d is more important as that is what lands in the man page.

@pszlazak
Copy link
Contributor Author

One more clarification - -q of course works for me in 7.29.0. Maybe if I read local man page instead of online manual I would be less confused. But docs/options-in-versions confused me even more.

@bagder
Copy link
Member

bagder commented Aug 23, 2023

If we want to be picky, that document states when the option was added. Not adjusted for bugs.

As you can see in your command line, --disable does not work but it also does not cause an error. That is because the command line option is recognized and supported, it just doesn't do the right thing.

If you would have tried an option that does not exist at all: curl --mooo localhost you instead get a proper error. I think that was true even back in ancient 7.29.0 days.

@pszlazak
Copy link
Contributor Author

If we want to be picky, that document states when the option was added. Not adjusted for bugs.

Yes, from this point of view all is fine.

As you can see in your command line, --disable does not work but it also does not cause an error.

The fact --disable was accepted was also part of the confusion. Today, out of the curiosity, I rebuilt 7.29.0 changing disable to disuble here:
https://github.com/curl/curl/blob/curl-7_29_0/src/tool_getparam.c#L239
So --disable was accepted because this alias is defined here.

I guess PR #11712 created by @jay is nice proposition. I will just need to learn new place to check details about command line options. Which btw is good-to-know resource.

@bagder - thank you for all your great work! 👏🏻

jay added a commit to jay/curl that referenced this issue Aug 23, 2023
@jay
disable.d: explain --disable not implemented prior to 7.50.0
22b1138 
Option -q/--disable was added in 5.0 but only -q was actually
implemented. Later --disable was implemented in e200034 (precedes
7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0).

Reported-by: pszlazak@users.noreply.github.com

Fixes curl#11710
Closes #xxxx
@bagder bagder closed this as completed in 89b3cbd Aug 28, 2023
ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
@jay @ptitSeb
disable.d: explain --disable not implemented prior to 7.50.0
be0efaa 
Option -q/--disable was added in 5.0 but only -q was actually
implemented. Later --disable was implemented in e200034 (precedes
7.49.0), but incorrectly, and fixed in 6dbc23c (precedes 7.50.0).

Reported-by: pszlazak@users.noreply.github.com

Fixes curl#11710
Closes curl#11712
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cmdline tool documentation
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

disable.d: explain --disable not implemented prior to 7.50.0 jay/curl
3 participants
@bagder @jay @pszlazak