From be22bb9ffdfc5960f8eefca2e9c55c6b8208b4fe Mon Sep 17 00:00:00 2001
From: Igor Kushnir <igorkuo@gmail.com>
Date: Thu, 24 Nov 2022 23:45:59 +0200
Subject: [PATCH] Prevent using temporary codec-for-locale UTF8 replacement

Utf8CodecForLocaleReplacer's temporary codec-for-locale replacements
cause race conditions in unrelated code that calls
QTextCodec::codecForLocale(). Implement and use a wrapper function that
calls QTextCodec::codecForLocale() while holding
Utf8CodecForLocaleReplacer's mutex lock to fix it.

f0b66f75071800b1cbc83e1e3015635df15e67da and this commit fix #1587.

Combine several chained QString::arg() calls into one in
EpwingBook::setErrorString(). A single call is more efficient and
prevents unintended replacement inside previous argument substitutions.
---
 epwing_book.cc |  6 +++---
 gddebug.cc     | 17 +++++++++++++++++
 gddebug.hh     |  4 ++++
 indexedzip.cc  |  2 +-
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/epwing_book.cc b/epwing_book.cc
index 1636f1af..9f2313ba 100644
--- a/epwing_book.cc
+++ b/epwing_book.cc
@@ -420,10 +420,10 @@ EpwingBook::~EpwingBook()
 
 void EpwingBook::setErrorString( QString const & func, EB_Error_Code code )
 {
+  QTextCodec * const localeCodec = gdCodecForLocale();
   error_string = QString( "EB \"%1\" function error: %2 (%3)" )
-                 .arg( func )
-                 .arg( QTextCodec::codecForLocale()->toUnicode( eb_error_string( code ) ) )
-                 .arg( QTextCodec::codecForLocale()->toUnicode( eb_error_message( code ) ) );
+                 .arg( func, localeCodec->toUnicode( eb_error_string( code ) ),
+                       localeCodec->toUnicode( eb_error_message( code ) ) );
 
   if( currentPosition.page != 0 )
     error_string += QString( " on page %1, offset %2" ).arg( QString::number( currentPosition.page ) )
diff --git a/gddebug.cc b/gddebug.cc
index 792038f8..a9765247 100644
--- a/gddebug.cc
+++ b/gddebug.cc
@@ -2,6 +2,7 @@
  * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */
 
 #include <QMutex>
+#include <QMutexLocker>
 #include <QTextCodec>
 #include <QString>
 #include "gddebug.hh"
@@ -13,6 +14,17 @@ namespace {
 class Utf8CodecForLocaleReplacer
 {
 public:
+  static QTextCodec * originalCodecForLocale()
+  {
+    if( !shouldReplaceCodecForLocale() )
+      return QTextCodec::codecForLocale();
+    // codecForLocaleMutex is locked while the UTF8 codec replaces the original codec.
+    // Thus calling QTextCodec::codecForLocale() while holding a lock is guaranteed to
+    // return the original codec, not its temporary UTF8 replacement.
+    QMutexLocker _( &codecForLocaleMutex );
+    return QTextCodec::codecForLocale();
+  }
+
   Utf8CodecForLocaleReplacer():
     replaceCodecForLocale( shouldReplaceCodecForLocale() ), localeCodec( 0 )
   {
@@ -68,3 +80,8 @@ void gdDebug(const char *msg, ...)
   }
   va_end(ap);
 }
+
+QTextCodec * gdCodecForLocale()
+{
+  return Utf8CodecForLocaleReplacer::originalCodecForLocale();
+}
diff --git a/gddebug.hh b/gddebug.hh
index cc1f0b74..72b07b77 100644
--- a/gddebug.hh
+++ b/gddebug.hh
@@ -3,6 +3,8 @@
 
 #include <QFile>
 
+class QTextCodec;
+
 #ifdef NO_CONSOLE
   #define GD_DPRINTF(...) do {} while( 0 )
   #define GD_FDPRINTF(...) do {} while( 0 )
@@ -27,6 +29,8 @@ void gdDebug(const char *, ...)
 #endif
 ;
 
+QTextCodec * gdCodecForLocale();
+
 extern QFile * logFilePtr;
 
 #endif // __GDDEBUG_HH_INCLUDED__
diff --git a/indexedzip.cc b/indexedzip.cc
index 6f06bd21..3480d654 100644
--- a/indexedzip.cc
+++ b/indexedzip.cc
@@ -133,7 +133,7 @@ bool IndexedZip::indexFile( BtreeIndexing::IndexedWords &zipFileNames, quint32 *
     // File seems to be a valid zip file
 
 
-    QTextCodec * localeCodec = QTextCodec::codecForLocale();
+    QTextCodec * const localeCodec = gdCodecForLocale();
 
     ZipFile::CentralDirEntry entry;