- Download Diff

Summary:

Fix critical crash when running on non utf-8 environment.

Review Request #7395 — Created June 8, 2015 and submitted June 10, 2015, 6:44 p.m.

Information
Owner:	bgolek
Repository:	RBTools
Branch:
Bugs:
Depends On:
Reviewers
Groups:	rbtools
People:

Description

When running on non UTF-8 environment with mercurial as repository RBTools crashes for every command with message:
'CRITICAL: 'utf8' codec can't decode byte 0xb3 in position 22: invalid start byte.'. This is caused by assumption in process.py execute method.

Now we are using sys.getfilesystemencoding() instead of assumpted 'utf-8'.

Testing Done

Running on non UTF-8 environment (Win7, x64 PL) does not cause error anymore.

Issues

Description	From	Last Updated
Blank line between these.	chipx86	June 8, 2015, 11:32 p.m.
The filesystem encoding doesn't seem like the "correct" thing here, since this is the output of the program. Does sys.getdefaultencoding() …	david	June 10, 2015, 6:36 p.m.

Tool: Pyflakes
Processed Files:
    rbtools/utils/process.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/process.py

Mind reworking the description into more of the format described here? https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/
That will help with getting a thorough understanding of the problem, its cause, its fix, and why the fix works. It will also help when bisecting commits later, and with preparation of release notes.

rbtools/utils/process.py (Diff revision 1)
Show all issues
```
Blank line between these.
```

Summary:

-	Avoid assuming utf-8 encoding of external command std output.
+	Fix critical crash when running on non utf-8 environment.

Description:

~		In my system (win 7 x64 PL) when invoking:
~		for line in execute(['hg', 'showconfig'], split_lines=True):
	~	When running on non UTF-8 environment with mercurial as repository RBTools crashes for every command with message:
	~	'CRITICAL: 'utf8' codec can't decode byte 0xb3 in position 22: invalid start byte.'. This is caused by assumption in process.py execute method.

~		I was getting:
	~	Now we are using sys.getfilesystemencoding() instead of assumpted 'utf-8'.
-		CRITICAL: 'utf8' codec can't decode byte 0xb3 in position 22: invalid start byte.
-
-		Fix: Using sys.getfilesystemencoding() instead if 'utf-8'

Testing Done:

~		"CRITICAL: 'utf8' codec can't decode byte 0xb3 in position 22: invalid start byte." not appearing for 'rbt diff'
	~	Running on non UTF-8 environment (Win7, x64 PL) does not cause error anymore.

Diff:

Revision 2 (+1 -1)

Show changes

rbtools/utils/process.py

Tool: Pyflakes
Processed Files:
    rbtools/utils/process.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/process.py

r2 of your diff doesn't seem like it is correct (it seems to only be the top commit instead of the full diff from origin/master)

rbtools/utils/process.py (Diff revision 2)

Show all issues

The filesystem encoding doesn't seem like the "correct" thing here, since this is the output of the program. Does sys.getdefaultencoding() return the right thing?

bgolek June 10, 2015, 4:42 a.m.

I think it does. Its better than assuming 'utf-8' (which is incorrect for windows family os [mbsc]).

Most correctly wrriten apps should use filesystem encoding to output, due to pipelining, streaming to file.

Win7: sys.getfilesystemencoding() returns mbsc
Debian: sys.getfilesystemencoding() returns UTF-8

sys.getdefaultencoding() always returns ascii, except when during app init it was changed by sys.setdefaultencoding() (which is 'discouraged'). See: http://stackoverflow.com/questions/3828723/why-we-need-sys-setdefaultencodingutf-8-in-a-py-script

david June 10, 2015, 6:36 p.m.

Sounds good, thanks for the explanation.

Diff:

Revision 3 (+6 -5)

Show changes

rbtools/utils/process.py

Tool: Pyflakes
Processed Files:
    rbtools/utils/process.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/process.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.7.x (df994eb)

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

Fix critical crash when running on non utf-8 environment.

Summary:

Description:

Testing Done:

Diff:

Diff:

Status: Closed (submitted)

Change Summary: