Replace uses of boost::trim* with locale-independent alternatives (#18130 rebased) #22859

1. 
   fanquake commented at 5:26 am on September 2, 2021: member

   This is #18130 rebased.

   TrimString is an existing alternative.

   Note TrimString uses " \f\n\r\t\v" as the pattern, which is consistent with the default behavior of std::isspace. See: https://en.cppreference.com/w/cpp/string/byte/isspace

2. Replace use of boost::trim use with locale-independent TrimString 93551862a1
3. Replace use of boost::trim_right with locale-independent TrimString

   Note the only use of readStdin is fed to DecodeHexTx, which fails in
   IsHex on non-hex characters as recorded in p_util_hexdigit.

   4bf18b089e
4. tests: Add TrimString(...) tests 696c76d660
5. fanquake added the label Refactoring on Sep 2, 2021
6. 
   fanquake commented at 5:27 am on September 2, 2021: member

   cc @practicalswift @l2a5b1
7. 
   jonatack commented at 4:52 pm on September 3, 2021: member

   Concept ACK
8. 
   jb55 commented at 9:46 pm on September 3, 2021: member

   utACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6
9. 
   practicalswift commented at 1:26 pm on September 4, 2021: contributor

   Very happy to see a reduction in our locale dependency!

   I think we’ll be able to have an empty KNOWN_VIOLATIONS list in test/lint/lint-locale-dependence.sh by the end of the year. That would be awesome! :)

   ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6

10. in src/httprpc.cpp:135 in 696c76d660

    130@@ -130,8 +131,7 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna
    131         return false;
    132     if (strAuth.substr(0, 6) != "Basic ")
    133         return false;
    134-    std::string strUserPass64 = strAuth.substr(6);
    135-    boost::trim(strUserPass64);
    136+    std::string strUserPass64 = TrimString(strAuth.substr(6));
    137     std::string strUserPass = DecodeBase64(strUserPass64);
    

    ---

    ---

    

    jonatack commented at 2:47 pm on September 4, 2021:

    9355186 nit, these can be const

    0-    std::string strUserPass64 = TrimString(strAuth.substr(6));
    1-    std::string strUserPass = DecodeBase64(strUserPass64);
    2+    const std::string strUserPass64 = TrimString(strAuth.substr(6));
    3+    const std::string strUserPass = DecodeBase64(strUserPass64);
    

11. 
    jonatack commented at 2:48 pm on September 4, 2021: member

    ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6
12. theStack approved
13. 
    theStack commented at 10:49 pm on September 4, 2021: member

    Code-review ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6

    Happy to also re-ACK if Jon’s suggestion of declaring strUserPass{64} as const (https://github.com/bitcoin/bitcoin/pull/22859#discussion_r702292854) is applied.

14. fanquake merged this on Sep 5, 2021
15. fanquake closed this on Sep 5, 2021

    ---

16. fanquake deleted the branch on Sep 5, 2021
17. sidhujag referenced this in commit 7265550bd6 on Sep 7, 2021
18. in src/bitcoin-tx.cpp:775 in 696c76d660

    771@@ -772,9 +772,7 @@ static std::string readStdin()
    772     if (ferror(stdin))
    773         throw std::runtime_error("error reading stdin");
    774 
    775-    boost::algorithm::trim_right(ret);
    776-
    777-    return ret;
    778+    return TrimString(ret);
    

    ---

    ---

    

    luke-jr commented at 4:36 pm on September 20, 2021:

    This SHOULD be a locale trim!

    ---

    

    sipa commented at 4:37 pm on September 20, 2021:

    Why? It isn’t trying to read native-language text.

    ---

    

    luke-jr commented at 5:17 pm on September 20, 2021:

    It’s interacting with the CLI, which is expected to behave in the user’s configured locale.

    ---

    

    sipa commented at 7:59 pm on September 20, 2021:

    When processing text, or other locale-formatted things like dates/…, I can see that. But this is just accepting hex-encoded tx data?
19. DrahtBot locked this on Oct 30, 2022

Contributors
fanquake jonatack jb55 practicalswift theStack luke-jr sipa

Labels
Refactoring