From kde-core-devel  Tue Aug 18 06:35:19 2009
From: Thiago Macieira <thiago () kde ! org>
Date: Tue, 18 Aug 2009 06:35:19 +0000
To: kde-core-devel
Subject: Re: Review Request: Fix sanitization of dbus path in KMainWindow
Message-Id: <200908180835.19443.thiago () kde ! org>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=125057789029764
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--nextPart1808441.gNj4iHRfuh"

--nextPart1808441.gNj4iHRfuh
Content-Type: Text/Plain;
  charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Matthew Woehlke wrote:
>Index: widgets/kmainwindow.cpp
>=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>--- 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 =3D=3D '_' || (c > 47 && c < 58);
>+}
>+

Why did you write this so cryptically?

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

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

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

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

=2D-=20
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

--nextPart1808441.gNj4iHRfuh
Content-Type: application/pgp-signature; name=signature.asc 
Content-Description: This is a digitally signed message part.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQBKikunM/XwBW70U1gRAqkjAJ0Yax8hM5hIQhm9pn4O1HB4evZPrgCfTInU
QlZAc0Gxhi4dQqbRXrLts1w=
=+82/
-----END PGP SIGNATURE-----

--nextPart1808441.gNj4iHRfuh--