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

Bad encoding experience #456

Closed
legendtang opened this issue Jun 6, 2016 · 3 comments
Closed

Bad encoding experience #456

legendtang opened this issue Jun 6, 2016 · 3 comments
Labels
fix Bug/defect, or a fix for such a problem help wanted

Comments

@legendtang
Copy link

Node version:

6.1.0

ShellJS version (the most recent version/Github branch you see the bug on):

0.7.0

Operating system:

Windows 10

Description of the bug:

Due to Node.js default behavior about encoding, all standard output will be transformed hard to utf-8. However, the default encoding of CMD is based on default lang of Windows, which means once you send command to non-utf8 terminal you will get non-utf8 output with utf-8 buffer string. That would probably irreversible, at least in my case in Chinese GB2312 (or Windows-936) encoding. Using iconv cannot revert the output again.

Example ShellJS command to reproduce the error:

sh.exec('dir', function(code, stdout, stderr) {
     console.log(stdout)
}

The output will be something like this:

 ������ D �еľ�û�б�ǩ��
 ���������

2016/06/06  13:55    <DIR>          .
2016/06/06  13:55    <DIR>          ..
2016/05/30  17:40                27 .babelrc
2016/06/03  13:37                95 .editorconfig
2016/05/30  17:40                29 .gitignore
2016/06/02  17:02             1,340 index.js
2016/05/31  11:27            93,035 jquery.min.js
2016/06/06  15:44    <DIR>          node_modules
2016/06/06  13:55             2,664 package.json
2016/05/19  13:07                39 README.md
2016/05/30  18:50    <DIR>          statics
2016/05/30  18:50    <DIR>          webpack-config
               7 ���ļ�         97,229 �ֽ�
               9 ��Ŀ¼ 311,094,530,048 �����ֽ�

If you manually convert to GBK, that will be:


2016/06/06  13:55    <DIR>          .
2016/06/06  13:55    <DIR>          ..
2016/05/30  17:40                27 .babelrc
2016/06/03  13:37                95 .editorconfig
2016/05/30  17:40                29 .gitignore
2016/06/02  17:02             1,340 index.js
2016/05/31  11:27            93,035 jquery.min.js
2016/06/06  15:44    <DIR>          node_modules
2016/06/06  13:55             2,664 package.json
2016/05/19  13:07                39 README.md
2016/05/30  18:50    <DIR>          statics
2016/05/30  18:50    <DIR>          webpack-config
               7 锟斤拷锟侥硷拷         97,229 锟街斤拷
               9 锟斤拷目录 311,094,530,048 锟斤拷锟斤拷锟街斤拷
@nfischer
Copy link
Member

nfischer commented Jun 6, 2016

@legendtang Thanks for reporting this! This is definitely a concern. As my environments are all English, I haven't experienced this issue and probably won't be able to write up a fix.

Internally, we rely on Node's child_process.exec API. Would this be better suited as a bug filed against Node itself or shelljs? If it's better to fix it here, that sounds good to me.

Would you be able to work on a PR? Also, if you can think of any way I can test this out, please let me know.

@nfischer nfischer added the fix Bug/defect, or a fix for such a problem label Jun 6, 2016
@pfdgithub
Copy link

interim solutions

var iconv = require('iconv-lite'); sh.exec('dir', { encoding: 'base64' }, function(code, stdout, stderr) { console.log(stdout); console.log(iconv.decode(iconv.encode(stdout, 'base64'), 'gb2312')); }

root causes

936 gb2312 ANSI/OEM Simplified Chinese (PRC, Singapore); Chinese Simplified (GB2312)

Code Pages
Code Page Identifiers

At the time we decided that it wasn't worthwhile to add support for legacy encodings to node.js.

Local Codepage support on Windows
Win + Russian locale + child_process.exec = ???
Local Codepage support on Windows

@nfischer
Copy link
Member

Would this be better suited as a bug filed against Node itself or shelljs? If it's better to fix it here, that sounds good to me.

I'm still unsure if this is our bug or Node's, and I don't know how to further investigate. I'm going to close this, but please ping this thread if this is still an issue and anyone has ideas for how to move this bug forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug/defect, or a fix for such a problem help wanted
Projects
None yet
Development

No branches or pull requests

3 participants