Opened 2 months ago

Closed 5 weeks 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:
Severity: Minor Keywords: encoding, windows, russian, nt-service, win32, review-group-20
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


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 2 months ago.
english_error_message.patch (498 bytes) - added by Vort 7 weeks ago.
tor_encoding_fixed.png (28.0 KB) - added by Vort 6 weeks ago.

Download all attachments as: .zip

Change History (14)

Changed 2 months ago by Vort

Attachment: tor_encoding_bug.png added


comment:1 Changed 2 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

comment:2 Changed 2 months ago by nickm

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

comment:3 Changed 7 weeks 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 7 weeks 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 7 weeks ago by Vort (previous) (diff)

Changed 7 weeks ago by Vort

Attachment: english_error_message.patch added

comment:5 Changed 7 weeks 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 6 weeks ago by nickm

Keywords: review-group-20 added

Creating review-group-20

comment:7 Changed 6 weeks ago by nickm

Owner: set to Vort
Status: needs_reviewassigned

comment:8 Changed 6 weeks ago by nickm

Status: assignedneeds_review

comment:9 Changed 6 weeks 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 6 weeks ago by Vort

Attachment: tor_encoding_fixed.png added

comment:11 Changed 5 weeks 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.