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

strerror: honor Unicode API choice on Windows #6005

Closed
wants to merge 1 commit into from

Conversation

@jblazquez
Copy link
Contributor

@jblazquez jblazquez commented Sep 24, 2020

The implementation of Curl_strerror on Windows was explicitly calling the ANSI version of the FormatMessage Win32 API function, regardless of whether curl was being built to use the Unicode APIs. Some Windows-like platforms (e.g. Xbox) don't have the ANSI versions of these functions, so curl should honor the choice of using Unicode APIs instead.

@@ -661,28 +662,19 @@ get_winapi_error(int err, char *buf, size_t buflen)

*buf = '\0';

#ifdef _WIN32_WCE

This comment has been minimized.

@jblazquez

jblazquez Sep 24, 2020
Author Contributor

There didn't seem to be any meaningful difference between the Windows CE version of this function and the regular Windows version, so I merged them.

@bagder bagder added the Windows label Sep 24, 2020
@bagder bagder requested review from jay and MarcelRaad Sep 24, 2020
Copy link
Member

@MarcelRaad MarcelRaad left a comment

👍 Thanks!

@bagder
Copy link
Member

@bagder bagder commented Sep 25, 2020

Thanks!

@bagder bagder closed this in bed5f84 Sep 25, 2020
jay added a commit that referenced this pull request Oct 1, 2020
Follow-up to bed5f84 from several days ago.

Ref: #6005
@jay
Copy link
Member

@jay jay commented Oct 1, 2020

The winapi error function is used by the other strerror functions like Curl_strerror and aren't those functions used to output to the console in ANSI, like for example if German is the OS language and the build of curl is Unicode then wouldn't that now be returning UTF-8 instead of ANSI from this function, and wouldn't that cause messed up console output?

@jblazquez
Copy link
Contributor Author

@jblazquez jblazquez commented Oct 1, 2020

@jay, I think you raise a good question. Previously, the use of FormatMessageA meant that the error message would be in the current codepage (which may or may not be UTF-8), and so printing it to the console would be safe. Printing UTF-8 instead of the current codepage could show the wrong characters.

If the main purpose of Curl_strerror is to retrieve an error message to print to the console, then replacing the new implementation with the old WinCE implementation that called FormatMessageW + wcstombs would be better. wcstombs should be converting from Unicode to the current codepage, and so the behavior should (hopefully) be the same as before these changes.

jay added a commit to jay/curl that referenced this pull request Oct 13, 2020
- Change get_winapi_error() to return the error string in the local
  codepage instead of UTF-8 encoding.

Two weeks ago bed5f84 fixed get_winapi_error() to work on xbox, but it
also changed the error string's encoding from local codepage to UTF-8.

We return the local codepage version of the error string because if it
is output to the user's terminal it will likely be with functions which
expect the local codepage (eg fprintf, failf, infof).

This is essentially a partial revert of bed5f84. The support for xbox
remains but the error string is reverted back to local codepage.

Ref: curl#6005

Closes #xxxx
bagder added a commit that referenced this pull request Oct 13, 2020
- Change get_winapi_error() to return the error string in the local
  codepage instead of UTF-8 encoding.

Two weeks ago bed5f84 fixed get_winapi_error() to work on xbox, but it
also changed the error string's encoding from local codepage to UTF-8.

We return the local codepage version of the error string because if it
is output to the user's terminal it will likely be with functions which
expect the local codepage (eg fprintf, failf, infof).

This is essentially a partial revert of bed5f84. The support for xbox
remains but the error string is reverted back to local codepage.

Ref: #6005

Reviewed-by: Marcel Raad
Closes #6065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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