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

Curl_strerror prefers POSIX over Windows system error codes #4550

Closed
jay opened this issue Nov 1, 2019 · 1 comment
Closed

Curl_strerror prefers POSIX over Windows system error codes #4550

jay opened this issue Nov 1, 2019 · 1 comment
Labels

Comments

@jay
Copy link
Member

@jay jay commented Nov 1, 2019

I did this

Richard Alcock reported on the mailing list that Windows system error code ERROR_SHARING_VIOLATION (32) is mapped to error string "Broken pipe", and he correctly hypothesized that Curl_strerror interpreted the error code as POSIX error code EPIPE (32).

curl/lib/strerror.c

Lines 678 to 686 in 9cd755e

/* 'sys_nerr' is the maximum errno number, it is not widely portable */
if(err >= 0 && err < sys_nerr)
strncpy(buf, strerror(err), max);
else {
if(!get_winsock_error(err, buf, max) &&
!FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM, NULL, err,
LANG_NEUTRAL, buf, (DWORD)max, NULL))
msnprintf(buf, max, "Unknown error %d (%#x)", err, err);
}

The issue is Curl_strerror on Windows prefers POSIX error strings over Windows system error strings. That is actually mostly correct because the calls we are recording the errors for are for POSIX /C runtime calls. However in the reported example the error code is from a Windows API call and recorded using GetLastError() (ie get the last windows system error code) so one would expect to get the mapped Windows system error string in that case.

I expected the following

2 options come to mind,

  1. Add a separate function like Curl_winsys_error that handles just winapi errors, for when we know for certain the error code we're passing in is definitely from windows system.

  2. Change the CreateFile/ReadFile calls to fopen/fread.

Windows system error codes that overlap (the ones less than 50) look to be file related except for ERROR_NOT_ENOUGH_MEMORY (8).

curl/libcurl version

curl 7.67.0-DEV (i386-pc-win32) libcurl/7.67.0-DEV Schannel WinIDN
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS Debug IDN IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI

operating system

Windows 7

Ref: 9c49824

@jay jay added the Windows label Nov 1, 2019
@gvanem
Copy link
Contributor

@gvanem gvanem commented Nov 2, 2019

That is actually mostly correct because the calls we are recording the errors for are for POSIX /C runtime calls.

Seems I was to blame for that back in 2004:
https://curl.haxx.se/mail/lib-2004-04/0094.html

I'm amazed it has worked fine for so long.

jay added a commit to jay/curl that referenced this issue Nov 10, 2019
- In all code call Curl_winapi_strerror instead of Curl_strerror when
  the error code is known to be from Windows GetLastError.

Curl_strerror prefers CRT error codes (errno) over Windows API error
codes (GetLastError) when the two overlap. When we know the error code
is from GetLastError it is more accurate to prefer the Windows API error
messages.

Reported-by: Richard Alcock

Fixes curl#4550
Closes #xxxx
@jay jay closed this in 5b22e1a Dec 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.