Fix handling of binary secrets in wxSecretStore with wxGTK

Use correct content type when creating binary secrets to prevent
libsecret from munging them.

Also update the sample to show more functionality.

Closes #24351.

Closes #24352.

include/wx/secretstore.h

@@ -40,7 +40,7 @@ public:
 
     // Creates a secret value from the given data.
     wxSecretValue(size_t size, const void *data)
-        : m_impl(NewImpl(size, data))
+        : m_impl(NewImpl(size, data, "application/octet-stream"))
     {
     }
 
@@ -48,7 +48,7 @@ public:
     explicit wxSecretValue(const wxString& secret)
     {
         const wxScopedCharBuffer buf(secret.utf8_str());
-        m_impl = NewImpl(buf.length(), buf.data());
+        m_impl = NewImpl(buf.length(), buf.data(), "text/plain");
     }
 
     wxSecretValue(const wxSecretValue& other);
@@ -90,7 +90,8 @@ public:
 private:
     // This method is implemented in platform-specific code and must return a
     // new heap-allocated object initialized with the given data.
-    static wxSecretValueImpl* NewImpl(size_t size, const void *data);
+    static wxSecretValueImpl*
+    NewImpl(size_t size, const void *data, const char* contentType);
 
     // This ctor is only used by wxSecretStore and takes ownership of the
     // provided existing impl pointer.

samples/secretstore/secretstore.cpp

@@ -25,6 +25,11 @@
 #include "wx/crt.h"
 #include "wx/log.h"
 
+#include <cctype>
+
+bool g_verbose_ {false};
+bool g_binary_ {false};
+
 bool Save(wxSecretStore& store, const wxString& service, const wxString& user)
 {
     char password[4096];
@@ -107,12 +112,50 @@ static bool PrintResult(bool ok)
     return ok;
 }
 
+static void PrintSecrets(const wxSecretValue& expected, const wxSecretValue& loaded)
+{
+    wxPrintf("Expected: size=%ld data=", expected.GetSize());
+    if (g_binary_)
+    {
+        wxPrintf("[");
+        size_t n = expected.GetSize();
+        const char* p = static_cast<const char *>(expected.GetData());
+        for (size_t i=0; i<n ;++i)
+            if (std::isprint(p[i]))
+                wxPrintf("%c", p[i]);
+            else
+                wxPrintf("\\x%hhx", p[i]);
+        wxPrintf("]\n");
+    }
+    else
+    {
+        wxPrintf("\"%s\"\n", expected.GetAsString());
+    }
+    wxPrintf("Loaded: size=%ld data=", loaded.GetSize());
+    if (g_binary_)
+    {
+        wxPrintf("[");
+        size_t n = loaded.GetSize();
+        const char* p = static_cast<const char *>(loaded.GetData());
+        for (size_t i=0; i<n ;++i)
+          if (std::isprint(p[i]))
+              wxPrintf("%c", p[i]);
+          else
+              wxPrintf("\\x%hhx", p[i]);
+        wxPrintf("]\n");
+    }
+    else
+    {
+        wxPrintf("\"%s\"\n", loaded.GetAsString());
+    }
+}
+
 bool SelfTest(wxSecretStore& store, const wxString& service)
 {
-    wxPrintf("Running the tests...\n");
+    wxPrintf("Running the tests with %s secrets...\n", g_binary_ ? "binary" : "text");
 
     const wxString userTest("test");
-    const wxSecretValue secret1(6, "secret");
+    const wxSecretValue secret1 = g_binary_ ? wxSecretValue(11, "secret\x1\x86\x2\x99\x3") : wxSecretValue("secret");
 
     wxPrintf("Storing the password:\t");
     bool ok = store.Save(service, userTest, secret1);
@@ -129,9 +172,13 @@ bool SelfTest(wxSecretStore& store, const wxString& service)
     ok = PrintResult(store.Load(service, user, secret) &&
                         user == userTest &&
                             secret == secret1);
+    if (!ok && g_verbose_)
+    {
+        PrintSecrets(secret1, secret);
+    }
 
     // Overwriting the password should work.
-    const wxSecretValue secret2(6, "privet");
+    const wxSecretValue secret2 = g_binary_ ? wxSecretValue(11, "privet\x1\x86\x2\x99\x3") : wxSecretValue("privet");
 
     wxPrintf("Changing the password:\t");
     if ( PrintResult(store.Save(service, user, secret2)) )
@@ -139,7 +186,13 @@ bool SelfTest(wxSecretStore& store, const wxString& service)
         wxPrintf("Reloading the password:\t");
         if ( !PrintResult(store.Load(service, user, secret) &&
                             secret == secret2) )
+        {
             ok = false;
+            if (g_verbose_)
+            {
+                PrintSecrets(secret2, secret);
+            }
+        }
     }
     else
         ok = false;
@@ -164,6 +217,24 @@ bool SelfTest(wxSecretStore& store, const wxString& service)
     return ok;
 }
 
+void usage(char **argv)
+{
+  wxFprintf(stderr,
+            "Usage: %s save <service> <user>\n"
+            "   or  %s {load|delete} <service>\n"
+            "   or  %s [options] selftest <service>\n"
+            "\n"
+            "Sample showing wxSecretStore class functionality.\n"
+            "Specify one of the commands to perform the corresponding\n"
+            "function call. The \"service\" argument is mandatory for\n"
+            "all commands, \"save\" also requires \"user\" and will\n"
+            "prompt for password.\n\n"
+            "options:\n"
+            "\t-v\trun verbose (possibly shows secrets on errors)\n"
+            "\t-b\trun selftest using binary secrets (otherwise uses text strings)\n\n",
+            argv[0], argv[0], argv[0]);
+}
+
 int main(int argc, char **argv)
 {
     // To complement the standard EXIT_{SUCCESS,FAILURE}.
@@ -176,20 +247,59 @@ int main(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
-    if ( argc < 2 ||
+    wxString operation;
-            argc != (argv[1] == wxString("save") ? 4 : 3) )
+    wxString service;
+    wxString user;
+
+    for (int arg=1; arg<argc ;++arg)
     {
+      if (argv[arg][0] == '-')
+      {
+        if (argv[arg][1] == 'v' && argv[arg][2] == '\0')
+          g_verbose_ = true;
+        else if (argv[arg][1] == 'b' && argv[arg][2] == '\0')
+          g_binary_ = true;
+        else
+        {
+          wxFprintf(stderr,
+                    "ERROR: Unknown switch : %s\n\n",
+                    argv[arg]);
+          usage(argv);
+          return EXIT_SYNTAX;
+        }
+      }
+      else if (operation.IsEmpty())
+      {
+        operation = argv[arg];
+      }
+      else if (service.IsEmpty())
+      {
+        service = argv[arg];
+      }
+      else if (operation == "save" && user.IsEmpty())
+      {
+        user = argv[arg];
+      }
+      else
+      {
         wxFprintf(stderr,
-                  "Usage: %s save <service> <user>\n"
+                  "ERROR: Unknown argument : %s\n\n",
-                  "   or  %s {load|delete|selftest} <service>\n"
+                  argv[arg]);
-                  "\n"
+        usage(argv);
-                  "Sample showing wxSecretStore class functionality.\n"
-                  "Specify one of the commands to perform the corresponding\n"
-                  "function call. The \"service\" argument is mandatory for\n"
-                  "all commands, \"save\" also requires \"user\" and will\n"
-                  "prompt for password.\n",
-                  argv[0], argv[0]);
         return EXIT_SYNTAX;
+      }
+    }
+
+    if (operation.IsEmpty() || service.IsEmpty())
+    {
+      if (operation.IsEmpty())
+        wxFprintf(stderr,
+                  "ERROR: Missing required operation argument\n\n");
+      else
+        wxFprintf(stderr,
+                  "ERROR: Missing required service argument\n\n");
+      usage(argv);
+      return EXIT_SYNTAX;
     }
 
     wxSecretStore store = wxSecretStore::GetDefault();
@@ -201,13 +311,17 @@ int main(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
-    const wxString operation = argv[1];
-    const wxString service = argv[2];
-
     bool ok;
     if ( operation == "save" )
     {
-        ok = Save(store, service, argv[3]);
+        if (user.IsEmpty())
+        {
+            wxFprintf(stderr,
+                      "ERROR: Missing required user argument\n\n");
+            usage(argv);
+            return EXIT_SYNTAX;
+        }
+        ok = Save(store, service, user);
     }
     else if ( operation == "load" )
     {
@@ -225,8 +339,9 @@ int main(int argc, char **argv)
     {
         wxFprintf(stderr,
                   "Unknown operation \"%s\", expected \"save\", \"load\" or "
-                  "\"delete\".\n",
+                  "\"delete\".\n\n",
                   operation);
+        usage(argv);
         return EXIT_SYNTAX;
     }
 

src/msw/secretstore.cpp

@@ -134,7 +134,10 @@ public:
 // ============================================================================
 
 /* static */
-wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data)
+wxSecretValueImpl*
+wxSecretValue::NewImpl(size_t size,
+                       const void *data,
+                       const char* WXUNUSED(contentType))
 {
     return new wxSecretValueGenericImpl(size, data);
 }

src/osx/core/secretstore.cpp

@@ -240,7 +240,10 @@ private:
 // ============================================================================
 
 /* static */
-wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data)
+wxSecretValueImpl*
+wxSecretValue::NewImpl(size_t size,
+                       const void *data,
+                       const char* WXUNUSED(contentType))
 {
     return new wxSecretValueGenericImpl(size, data);
 }

src/unix/secretstore.cpp

@@ -68,15 +68,10 @@ private:
 class wxSecretValueLibSecretImpl : public wxSecretValueImpl
 {
 public:
-    // Create a new secret value.
+    // Create a new secret value for the given content type.
-    //
+    wxSecretValueLibSecretImpl(size_t size, const void* data, const char* contentType)
-    // Notice that we have to use text/plain as content type and not
-    // application/octet-stream which would have been more logical because
-    // libsecret accepts only valid UTF-8 strings for the latter, while our
-    // data is not necessarily UTF-8 (nor even text at all...).
-    wxSecretValueLibSecretImpl(size_t size, const void* data)
         : m_value(secret_value_new(static_cast<const gchar*>(data), size,
-                                   "text/plain"))
+                                   contentType))
     {
     }
 
@@ -362,9 +357,12 @@ const char* wxSecretStoreLibSecretImpl::FIELD_USER = "user";
 // ============================================================================
 
 /* static */
-wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data)
+wxSecretValueImpl*
+wxSecretValue::NewImpl(size_t size,
+                       const void *data,
+                       const char* contentType)
 {
-    return new wxSecretValueLibSecretImpl(size, data);
+    return new wxSecretValueLibSecretImpl(size, data, contentType);
 }
 
 /* static */