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

os.hostname() return garbled code on windows if the host name contains non ascii characters #38255

Closed
eagleliang opened this issue Apr 16, 2021 · 6 comments
Labels
confirmed-bug Issues with confirmed bugs. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.

Comments

@eagleliang
Copy link

  • 12.14.0 -- latest version:
  • Window 10:
  • Subsystem:

What steps will reproduce the bug?

  1. windows 7/8/10
  2. set pc's name to ANDREW-ü-LAPTOP
  3. make sure 'Use unicode UTF-8 support' is not enable in windows language setting
  4. in node CLI, invoke os.hostname(), it returns ANDREW-�-LAPTOP

How often does it reproduce? Is there a required condition?

Always happen.

What is the expected behavior?

ANDREW-ü-LAPTOP

What do you see instead?

ANDREW-�-LAPTOP

Additional information

the fix of #27848 would work fine on linux/macOs. But on windows, libuv's implement does not decoding the string which return be windows API, which cause this bug.

@eagleliang
Copy link
Author

@addaleax @cjihrig I have fixed this locally and want to Push request. But I'm not sure about the stratagy of this PR, since the root cause is in deps/uv/src/win/util.c.

Should I push this to libuv/libuv first. And then merge the files back to deps/uv after the PR merged in libuv/libuv ?

@RaisinTen
Copy link
Contributor

Yes, you may make a PR at libuv with your change and once that lands, the code in Node.js too will be updated with your change when deps/uv is upgraded with a new release.

@Ayase-252 Ayase-252 added libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform. labels Apr 16, 2021
@eagleliang
Copy link
Author

eagleliang commented May 14, 2021

@Ayase-252 Can I make a PR to update deps/uv from libuv as the my PR already be merged in it.
Because our project awaiting this update in nodejs

@Ayase-252
Copy link
Member

Ayase-252 commented May 14, 2021

@eagleliang

Node.js will update libuv dep when libuv releases a new version in normal workflow. I'm not sure how to handle this case if the patch is urge for you.

cc @cjihrig

@cjihrig
Copy link
Contributor

cjihrig commented May 14, 2021

Yes, we should wait for a new libuv release.

@jasnell jasnell added the confirmed-bug Issues with confirmed bugs. label May 14, 2021
@Ayase-252
Copy link
Member

We have updated libuv to v1.42.0 in #39525.

It should be resolved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants