Review Request: Fix sanitization of dbus path in KMainWindow

[Thiago Macieira]

Matthew Woehlke wrote:
>Index: widgets/kmainwindow.cpp
>===================================================================
>--- widgets/kmainwindow.cpp     (revision 1012587)
>+++ widgets/kmainwindow.cpp     (working copy)
>@@ -306,6 +306,12 @@
>      return false;
>  }
>
>+static bool isIdentifier(char c)
>+{  // the order btw is [a-zA-Z_0-9]
>+    return (c > 96 && c < 123) || (c > 64 && c < 91) ||
>+               c == '_' || (c > 47 && c < 58);
>+}
>+

Why did you write this so cryptically?

>-    QString pathname = q->objectName();
>+    QString pathname = QString( q->objectName().toAscii() );
>      // Clean up for dbus usage: any non-alphanumeric char should be
>turned into '_'
>      const int len = pathname.length();
>      for ( int i = 0; i < len; ++i ) {
>-        if ( !( pathname[i].isLetter() || pathname[i].isDigit() ) )
>+        if ( !isIdentifier( pathname[i].toAscii() ) )
>              pathname[i] = QLatin1Char('_');
>      }
>      pathname = '/' + qApp->applicationName() + '/' + pathname;

From qdbusutil.cpp:
static inline bool isValidCharacterNoDash(const QChar &c)
{
    register ushort u = c.unicode();
    return (u >= 'a' && u <= 'z')
            || (u >= 'A' && u <= 'Z')
            || (u >= '0' && u <= '9')
            || (u == '_');
}

Then use this function with your loop above. There's no need for toAscii 
anywhere:
>-    QString pathname = q->objectName();
>+     QString pathname = '/' + qApp->applicationName() + '/' 
>+     + q->objectName();
>      // Clean up for dbus usage: any non-alphanumeric char should be
>turned into '_'
>      const int len = pathname.length();
>      for ( int i = 0; i < len; ++i ) {
>-        if ( !( pathname[i].isLetter() || pathname[i].isDigit() ) )
>+        if ( pathname != QLatin1Char('/') && !isValidCharacterNoDash( 
pathname[i] ) )
>              pathname[i] = QLatin1Char('_');
>      }
>-     pathname = '/' + qApp->applicationName() + '/' + pathname;

I've also moved the application name above the cleanup, since it needs to 
be cleaned up too. That required checking for /.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
  Senior Product Manager - Nokia, Qt Development Frameworks
      PGP/GPG: 0x6EF45358; fingerprint:
      E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358

Qt Developer Days 2009 | Registration Now Open!
Munich, Germany: Oct 12 - 14     San Francisco, California: Nov 2 - 4
      http://qt.nokia.com/qtdevdays2009
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090818/ba18f494/attachment.sig>