Skip to content

wcurl: fix missing argument crash for options#98

Merged
sergiodj merged 1 commit into
curl:mainfrom
arpitguptagithub:fix-missing-arg-crash
May 1, 2026
Merged

wcurl: fix missing argument crash for options#98
sergiodj merged 1 commit into
curl:mainfrom
arpitguptagithub:fix-missing-arg-crash

Conversation

@arpitguptagithub

Copy link
Copy Markdown
Contributor

This PR adds input validation checks before shift is called for the --curl-options and -o/-O/--output parameters.

Currently, if a user passes --curl-options or -o as the final argument without providing an actual value, the script blindly calls shift and then attempts to read ${1}. Because the script correctly uses set -eu, reading an unbound variable immediately crashes the program with an ugly shell interpreter error (unbound variable or parameter not set)....

instead of a helpful user-facing error.

This is a small patch .... which catches the missing parameter and gracefully reports it using the error() function.

small test is also added here testMissingArgumentError to tests.sh to explicitly verify behavior when an argument is omitted.

@sergiodj sergiodj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! There are some minor nits regarding the error message format. Let's get that fixed first :-).

Comment thread wcurl Outdated
Comment thread wcurl Outdated
Comment thread tests/tests.sh Outdated
Comment thread tests/tests.sh Outdated
@arpitguptagithub arpitguptagithub force-pushed the fix-missing-arg-crash branch from 20a8428 to 506a7cf Compare May 1, 2026 23:39
@arpitguptagithub arpitguptagithub force-pushed the fix-missing-arg-crash branch from 506a7cf to 1849ef2 Compare May 1, 2026 23:45

@sergiodj sergiodj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sergiodj sergiodj merged commit 2ddb7b8 into curl:main May 1, 2026
4 checks passed
@soliton-

soliton- commented May 5, 2026

Copy link
Copy Markdown

Note that -z checks for empty and not whether an argument was given so the error is misleading if an empty argument was given. The wcurl argument parsing loop is similarly broken by checking for empty instead of checking $#.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants