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: improve --stderr handling #10673

Closed
wants to merge 3 commits into from
Closed

tool: improve --stderr handling #10673

wants to merge 3 commits into from

Conversation

jay
Copy link
Member

@jay jay commented Mar 4, 2023

  • freopen stderr with the user-specified file (--stderr file) instead of using a separate 'errors' stream.

  • Override stderr macro in tool_setup as global variable tool_stderr.

Both freopen and overriding the stderr macro are necessary because if the user-specified filename is "-" then stdout is assigned to tool_stderr and no freopen takes place.

Ref: #10491

Closes #xxxx


Note --stderr maps to libcurl's CURLOPT_STDERR but they are documented differently:

--stderr :
"Redirect all writes to stderr"

CURLOPT_STDERR :
"when showing the progress meter and displaying CURLOPT_VERBOSE data"

If the curl tool has freopened stderr as a file (--stderr file) then libcurl stderr output will also go to the file. OTOH if stdout was specified (--stderr -) then libcurl writes to stderr go to actual stderr and just the criteria above is written to stdout. IMO the distinction is moot since in libcurl we only write to stderr for developer debug output.

}
else
global->errors = stdout;
tool_set_stderr_file(nextarg);

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This argument to a file access function is derived from [user input (a command-line argument)](1) and then passed to tool_set_stderr_file(filename), which calls fopen(__filename).

/* precheck that filename is accessible to lessen the chance that the
subsequent freopen will fail. */
fp = fopen(filename, FOPEN_WRITETEXT);

Check failure

Code scanning / CodeQL

File created without restricting permissions

A file may be created here with mode 0666, which would make it world-writable.
@jay jay force-pushed the improve_stderr branch 2 times, most recently from 675799e to af1df98 Compare March 6, 2023 05:51
@bagder
Copy link
Member

bagder commented Mar 6, 2023

I'm not sure I see how this simplifies or improves the current situation, but if you think so then I am fine with it.

@jay
Copy link
Member Author

jay commented Mar 7, 2023

It's an improvement to fprintf(stderr... rather than a separate error stream, when all stderr output is supposed to be redirected, since in the latter case we forgot at least once which led to a bug. It's failing some CI so I'm still working on it.

- freopen stderr with the user-specified file (--stderr file) instead of
  using a separate 'errors' stream.

- Override stderr macro in tool_setup as global variable tool_stderr.

Both freopen and overriding the stderr macro are necessary because if
the user-specified filename is "-" then stdout is assigned to
tool_stderr and no freopen takes place. See the PR for more information.

Ref: curl#10491

Closes #xxxx
@jay jay closed this in 2f17a9b Mar 12, 2023
@jay jay deleted the improve_stderr branch March 12, 2023 06:25
GerHobbelt added a commit to GerHobbelt/thirdparty-curl that referenced this pull request Mar 15, 2023
* tool: improve --stderr handling

- freopen stderr with the user-specified file (--stderr file) instead of
  using a separate 'errors' stream.

- In tool_setup.h override stdio.h's stderr macro as global variable
  tool_stderr.

Both freopen and overriding the stderr macro are necessary because if
the user-specified filename is "-" then stdout is assigned to
tool_stderr and no freopen takes place. See the PR for more information.

Ref: curl/curl#10491

Closes curl/curl#10673
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
- freopen stderr with the user-specified file (--stderr file) instead of
  using a separate 'errors' stream.

- In tool_setup.h override stdio.h's stderr macro as global variable
  tool_stderr.

Both freopen and overriding the stderr macro are necessary because if
the user-specified filename is "-" then stdout is assigned to
tool_stderr and no freopen takes place. See the PR for more information.

Ref: curl#10491

Closes curl#10673
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