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

improved debug logging for cfilters #10271

Closed
wants to merge 5 commits into from
Closed

improved debug logging for cfilters #10271

wants to merge 5 commits into from

Conversation

icing
Copy link
Contributor

@icing icing commented Jan 10, 2023

New curl_log.[ch] to keep failf/infof and debug logging implementations.

  • new functions and macros for cfilter debugging
  • set CURL_DEBUG with names of cfilters where debug logging should be enabled
  • use GNUC attribute to enable printf format checks during compile

Usage on a debug build:

> CURL_DEBUG=happy-eyeballs,ssl curl -v https://curl.se 
* STATE: INIT => CONNECT handle 0x7f9a0c80ae08; line 1908 (connection #-5000)
* Added connection 0. The cache now contains 1 members
* STATE: CONNECT => RESOLVING handle 0x7f9a0c80ae08; line 1954 (connection #0)
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0* STATE: RESOLVING => CONNECTING handle 0x7f9a0c80ae08; line 2028 (connection #0)
* [CONN-0-HAPPY-EYEBALLS] ipv6 starting
*   Trying [2a04:4e42:a00::347]:443...
* [CONN-0-HAPPY-EYEBALLS] ipv6 connect -> 0, connected=0
* [CONN-0-HAPPY-EYEBALLS] ipv6 connect -> 0, connected=1
...
* [CONN-0-SSL] bio_cf_out_write(len=517) -> 517, err=0
...

Tried to make the macros performant and work for all compilers. Let's see what CI says.

lib/curl_log.c Dismissed
case CURLINFO_HEADER_OUT:
case CURLINFO_HEADER_IN:
fwrite(s_infotype[type], 2, 1, data->set.err);
fwrite(ptr, size, 1, data->set.err);

Check failure

Code scanning / CodeQL

Cleartext storage of sensitive information in file High

This write into file 'err' may contain unencrypted data from
this source.
.
This write into file 'err' may contain unencrypted data from
this source.
.
This write into file 'err' may contain unencrypted data from
this source.
.
This write into file 'err' may contain unencrypted data from
this source.
.
lib/http2.c Show resolved Hide resolved
…ons.

    - new functions and macros for cfilter debugging
    - set CURL_DEBUG with names of cfilters where debug logging should be enabled
    - use GNUC __attribute__ to enable printf format checks during compile
CF_DEBUGF(infof(data, CFMSG(cf, "recv(len=%zu) -> %d, err=%d"),
len, (int)nread, *err));
DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
*err));
Copy link
Member

Choose a reason for hiding this comment

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

I appreciate why this may be necessary temporarily, and useful, but even for me this is sometimes too verbose. It looks like this PR is going to change CF logging so that it cannot be disabled on DEBUGBUILDs any longer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Theses statements will not produce output, unless you enable the logging for a certain cfilter type.

CURL_DEBUG=tcp curl -v url

will enable the logging of the 'TCP' socket cfilter.

The idea here is to have logging available, should you wish, without recompiling the debug-enabled curl.

Copy link
Member

Choose a reason for hiding this comment

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

Neat. We will need documentation explaining how it works.

@bagder
Copy link
Member

bagder commented Jan 12, 2023

Thanks!

@bagder bagder closed this in db91dbb Jan 12, 2023
@icing icing deleted the logging branch January 16, 2023 11:19
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.

None yet

3 participants