Windows crashes for -wallet=你好 #13754

1. 
   Sjors commented at 3:55 pm on July 24, 2018: member

   Invalid characters for a wallet name, e.g. bitcoin-qt -wallet=你好 used to result in an error like this

   But now it just crashes:

   I didn’t do a git bisect, but it broke somewhere this year :-)

2. 
   achow101 commented at 4:05 pm on July 24, 2018: member

   Reported in #13103 and possible fix in #13426
3. MarcoFalke added this to the milestone 0.17.0 on Jul 24, 2018
4. MarcoFalke added the label Wallet on Jul 24, 2018
5. MarcoFalke added the label Windows on Jul 24, 2018
6. 
   Sjors commented at 5:11 pm on July 24, 2018: member

   @achow101 this is worse than what led to 13426:

   -wallet=d:\錢包 does works

   It doesn’t work at all (for me).

   But perhaps that PR makes this problem go away too. Will test later.

7. 
   ken2812221 commented at 11:04 pm on July 24, 2018: contributor

   ~I don’t think this can be fixed by #13426, our bdb and leveldb api does not support unicode on Windows.~

   ~@Sjors You can change your language setting to Chinese by this to make -wallet=你好 work.~

   Now It does not require those steps

8. 
   Sjors commented at 8:21 am on July 25, 2018: member

   For the scope of this ticket, I’m happy if unicode wallet names throw an error message, instead of crashing. That’s assuming this was always the case, because otherwise users get locked out of old wallets (or they’d have to rename the file).

   I think unicode already works on my machine (a6f00ce66f):

   It’s just passing characters in at launch that seems broken.

   Although it won’t let me add such characters to bitcoin.conf either:

   After save:

   This might be expect behavior; at least Windows clearly warns against doing this.

9. 
   Sjors commented at 9:30 am on July 25, 2018: member

   Just tried #13426 and it also crashes (rather than throw an error).
10. 
    Sjors commented at 9:53 am on July 25, 2018: member

    Actually it’s more complicated. As the page @ken2812221 links to points out, there are applications that don’t support Unicode, and for those Windows can switch the language (system wide). That probably includes the shel, the launch-from-search feature I used and the ANSI encoded file Notepad complained about.

    It’s a bit hard to find in Windows 10, maybe this helps:

    Don’t worry, this will not change your entire desktop into Mandarin. But now you can do this without a crash:

    This neatly creates the wallet:

    But the RPC gets confused:

    And that’s where #13426 shows its magic:

11. 
    MarcoFalke commented at 3:51 pm on August 1, 2018: member

    Confirmed that the issue still exists on current master. Also with an assertion hit:

    

12. 
    MarcoFalke commented at 12:50 pm on August 3, 2018: member

    This seems to be the cause: https://github.com/bitcoin/bitcoin/commit/26c06f24e5dcc32a7599abb8d670d22993c82bc2#diff-3b23ce4d86903a432863cfb3a1f1e547L238

    I am not sure how to fix this, since I can’t see the exception that is thrown on this file name.

    We might just add to the release notes that windows users should only use ascii-filenames that are valid under windows?

13. 
    Sjors commented at 5:24 pm on August 3, 2018: member

    Sounds like that second check shouldn’t have been removed. Maybe we can put it back, but only apply SanitizeString the filename component of the path?
14. 
    ken2812221 commented at 7:41 pm on August 3, 2018: contributor

    SanitizeString seem to make the wallet filename be ASCII only. Do we want to do that?
15. 
    Sjors commented at 7:58 pm on August 3, 2018: member

    If it was that way before, that seems safest. We can allow UTF-8 names in your PR.
16. 
    Sjors commented at 8:01 pm on August 3, 2018: member

    Oh wait, I think that commit is already in 0.16? Then we can’t put the constraint back because that could lock out wallets.
17. 
    MarcoFalke commented at 8:10 pm on August 3, 2018: member

    No, it wasn’t backported IIRC.
18. MarcoFalke removed this from the milestone 0.17.0 on Aug 5, 2018
19. MarcoFalke closed this on Aug 9, 2018

    ---

20. MarcoFalke referenced this in commit 8eb9870052 on Aug 9, 2018
21. PastaPastaPasta referenced this in commit a992e025f1 on Feb 2, 2021
22. PastaPastaPasta referenced this in commit ce89dc7c58 on Feb 3, 2021
23. PastaPastaPasta referenced this in commit b08e60e529 on Feb 4, 2021
24. DrahtBot locked this on Sep 8, 2021
25. gades referenced this in commit 517db54b3e on Mar 22, 2022

Contributors
Sjors achow101 ken2812221 MarcoFalke

Labels
Wallet Windows