Opened 6 months ago

Closed 4 months ago

#22520 closed defect (fixed)

Incorrect encoding for error message

Reported by: Vort Owned by: Vort
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.3.1.2-alpha
Severity: Minor Keywords: encoding, windows, russian, nt-service, win32, review-group-20
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When you try to run "tor --service install" in russian Windows 7 x64 SP1 without administrative priviliges, you will get following error: "OpenSCManager() failed : ╬Єърчрэю т фюёЄєях."
But this is incorrect. Correct message should be "OpenSCManager() failed : Отказано в доступе.".

Child Tickets

Attachments (3)

tor_encoding_bug.png (29.4 KB) - added by Vort 6 months ago.
Screenshot
english_error_message.patch (498 bytes) - added by Vort 5 months ago.
tor_encoding_fixed.png (28.0 KB) - added by Vort 5 months ago.

Download all attachments as: .zip

Change History (14)

Changed 6 months ago by Vort

Attachment: tor_encoding_bug.png added

Screenshot

comment:1 Changed 5 months ago by teor

I wonder if this happens because our Windows builds don't -DUNICODE by default?

Or I wonder if we are getting the wrong language ID: maybe we should just let Windows decide for us:

If you pass in zero, FormatMessage looks for a message for LANGIDs in the following order:

    Language neutral
    Thread LANGID, based on the thread's locale value
    User default LANGID, based on the user's default locale value
    System default LANGID, based on the system default locale value
    US English

https://msdn.microsoft.com/en-us/library/windows/desktop/ms679351(v=vs.85).aspx

comment:2 Changed 5 months ago by nickm

Keywords: nt-service win32 added
Milestone: Tor: 0.3.1.x-finalTor: unspecified

comment:3 Changed 5 months ago by Vort

What I can say now is that Windows have two codepages for russian language : cp866 (which is MS-DOS legacy; used in console) and cp1251 (which is used for non-Unicode GUI).
Tor, probably, selects wrong one for console output.

comment:4 Changed 5 months ago by Vort

The easiest solution is to change error message language to English.
This should not be a problem, since all other messages in Tor are also in English.

With this change, error message will look like:
"OpenSCManager() failed : Access is denied."

Here is the patch for this variant: attachment:english_error_message.patch.

Last edited 5 months ago by Vort (previous) (diff)

Changed 5 months ago by Vort

Attachment: english_error_message.patch added

comment:5 Changed 5 months ago by teor

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Status: newneeds_review

I think this is a good and simple change, but I can't test it.
Can someone else who isn't the patch author make sure it works?

Also, I think this is a good idea in general because it means that we don't log the user's language settings. (Which isn't that important on a relay, but it's still good.)

comment:6 Changed 5 months ago by nickm

Keywords: review-group-20 added

Creating review-group-20

comment:7 Changed 5 months ago by nickm

Owner: set to Vort
Status: needs_reviewassigned

comment:8 Changed 5 months ago by nickm

Status: assignedneeds_review

comment:9 Changed 5 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final
Status: needs_reviewmerge_ready

I think this is correct and mergeable based on my understanding of the msdn documents. I've put it in a branch as bug22520_031.

Vort, did you test this?

Changed 5 months ago by Vort

Attachment: tor_encoding_fixed.png added

comment:11 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Okay; merged to maint-0.3.1 and forward. Thank you again!

Note: See TracTickets for help on using tickets.