Work-around for broken MS device

Work-around for broken MS device

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Heya,

As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
The Microsoft Wireless Notebook Presenter Mouse 8000 has its name in
ISO-8859-1 instead of UTF-8, as required by the BT spec.

I've implemented a small work-around. This isn't very invasive, IMO, as
we already do UTF-8 checks.

In my tests, this makes the mouse show up as:
Microsoft® Wireless Notebook Presenter Mouse 8000

Cheers

[-- Attachment #2: bluez-try-utf8-harder.patch --]
[-- Type: text/x-patch, Size: 766 bytes --]

diff --git a/src/security.c b/src/security.c
index a61d75f..75908ba 100644
--- a/src/security.c
+++ b/src/security.c
@@ -600,8 +600,16 @@ static inline void remote_name_information(int dev, bdaddr_t *sba, void *ptr)
 		memcpy(name, evt->name, 248);
 		/* It's ok to cast end between const and non-const since
 		 * we know it points to inside of name which is non-const */
-		if (!g_utf8_validate(name, -1, (const char **) &end))
-			*end = '\0';
+		if (!g_utf8_validate(name, -1, (const char **) &end)) {
+			char *utf8_name;
+
+			utf8_name = g_convert(name, -1, "UTF-8", "ISO-8859-1", NULL, NULL, NULL);
+			if (utf8_name) {
+				memcpy(name, utf8_name, 248);
+				g_free(utf8_name);
+			} else
+				*end = '\0';
+		}
 		write_device_name(sba, &dba, name);
 	}
 

Re: [PATCH] Re: Work-around for broken MS device

[Marcel Holtmann]

Hi Bastien,

 
> > As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
> > The Microsoft Wireless Notebook Presenter Mouse 8000 has its name in
> > ISO-8859-1 instead of UTF-8, as required by the BT spec.
> > 
> > I've implemented a small work-around. This isn't very invasive, IMO, as
> > we already do UTF-8 checks.
> > 
> > In my tests, this makes the mouse show up as:
> > Microsoft® Wireless Notebook Presenter Mouse 8000
> 
> Attached is patch in the proper format.

I see the need for this one, but just a failing UTF-8 check to assume we
are using ISO-8859-1 is not good enough. Since the mouse has a DID
record, can we tie this together with the VID and PID?

Regards

Marcel

[PATCH] Re: Work-around for broken MS device

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

On Thu, 2009-03-05 at 18:48 +0000, Bastien Nocera wrote:
> Heya,
> 
> As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
> The Microsoft Wireless Notebook Presenter Mouse 8000 has its name in
> ISO-8859-1 instead of UTF-8, as required by the BT spec.
> 
> I've implemented a small work-around. This isn't very invasive, IMO, as
> we already do UTF-8 checks.
> 
> In my tests, this makes the mouse show up as:
> Microsoft® Wireless Notebook Presenter Mouse 8000

Attached is patch in the proper format.

Cheers

[-- Attachment #2: 0001-Try-harder-to-get-a-UTF-8-name-for-devices.patch --]
[-- Type: text/x-patch, Size: 1470 bytes --]

>From eccafa2920607e9d702677dfce1098d63d13f1be Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Wed, 18 Mar 2009 17:24:36 +0000
Subject: [PATCH] Try harder to get a UTF-8 name for devices
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

The Microsoft Wireless Notebook Presenter Mouse 8000 has its name in
ISO-8859-1 instead of UTF-8, as required by the BT spec.

So we try to convert the name to UTF-8 from ISO-8859-1 instead of
just truncating the name. The mouse now shows up as:
Microsoft® Wireless Notebook Presenter Mouse 8000
---
 src/security.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/security.c b/src/security.c
index a61d75f..75908ba 100644
--- a/src/security.c
+++ b/src/security.c
@@ -600,8 +600,16 @@ static inline void remote_name_information(int dev, bdaddr_t *sba, void *ptr)
 		memcpy(name, evt->name, 248);
 		/* It's ok to cast end between const and non-const since
 		 * we know it points to inside of name which is non-const */
-		if (!g_utf8_validate(name, -1, (const char **) &end))
-			*end = '\0';
+		if (!g_utf8_validate(name, -1, (const char **) &end)) {
+			char *utf8_name;
+
+			utf8_name = g_convert(name, -1, "UTF-8", "ISO-8859-1", NULL, NULL, NULL);
+			if (utf8_name) {
+				memcpy(name, utf8_name, 248);
+				g_free(utf8_name);
+			} else
+				*end = '\0';
+		}
 		write_device_name(sba, &dba, name);
 	}
 
-- 
1.6.0.6

Re: [PATCH] Re: Work-around for broken MS device

[Bastien Nocera]

On Sat, 2009-03-14 at 07:21 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
>  
> > > As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
> > > The Microsoft Wireless Notebook Presenter Mouse 8000 has its name in
> > > ISO-8859-1 instead of UTF-8, as required by the BT spec.
> > > 
> > > I've implemented a small work-around. This isn't very invasive, IMO, as
> > > we already do UTF-8 checks.
> > > 
> > > In my tests, this makes the mouse show up as:
> > > Microsoft® Wireless Notebook Presenter Mouse 8000
> > 
> > Attached is patch in the proper format.
> 
> I see the need for this one, but just a failing UTF-8 check to assume we
> are using ISO-8859-1 is not good enough. Since the mouse has a DID
> record, can we tie this together with the VID and PID?

In those kind of cases, we can only guess what the encoding would be, we
can't detect those encodings reliably. Given that we fallback to the old
behaviour when the device, and that ISO-8859-X corresponds 1-1 with
UTF-8 for the ASCII characters, I don't think it's such a stretch to
think that American companies (who'd be using ISO-8859-1) would be the
worst offenders for this sort of mistake.

I don't think special casing only this device would be a good idea. As I
mentioned, I reproduced the bug by making my computer, running BlueZ,
adopt that name.

Re: [PATCH] Re: Work-around for broken MS device

[Marcel Holtmann]

Hi Bastien,

>>>> As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
>>>> The Microsoft Wireless Notebook Presenter Mouse 8000 has its name  
>>>> in
>>>> ISO-8859-1 instead of UTF-8, as required by the BT spec.
>>>>
>>>> I've implemented a small work-around. This isn't very invasive,  
>>>> IMO, as
>>>> we already do UTF-8 checks.
>>>>
>>>> In my tests, this makes the mouse show up as:
>>>> Microsoft® Wireless Notebook Presenter Mouse 8000
>>>
>>> Attached is patch in the proper format.
>>
>> I see the need for this one, but just a failing UTF-8 check to  
>> assume we
>> are using ISO-8859-1 is not good enough. Since the mouse has a DID
>> record, can we tie this together with the VID and PID?
>
> In those kind of cases, we can only guess what the encoding would  
> be, we
> can't detect those encodings reliably. Given that we fallback to the  
> old
> behaviour when the device, and that ISO-8859-X corresponds 1-1 with
> UTF-8 for the ASCII characters, I don't think it's such a stretch to
> think that American companies (who'd be using ISO-8859-1) would be the
> worst offenders for this sort of mistake.
>
> I don't think special casing only this device would be a good idea.  
> As I
> mentioned, I reproduced the bug by making my computer, running BlueZ,
> adopt that name.

can we just fallback to ASCII only? Looks better to me than assuming  
Latin-1. We replace invalid characters with spaces in that case.

Regards

Marcel

Re: [PATCH] Re: Work-around for broken MS device

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

On Mon, 2009-03-23 at 16:45 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
> >>>> As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
> >>>> The Microsoft Wireless Notebook Presenter Mouse 8000 has its name  
> >>>> in
> >>>> ISO-8859-1 instead of UTF-8, as required by the BT spec.
> >>>>
> >>>> I've implemented a small work-around. This isn't very invasive,  
> >>>> IMO, as
> >>>> we already do UTF-8 checks.
> >>>>
> >>>> In my tests, this makes the mouse show up as:
> >>>> Microsoft® Wireless Notebook Presenter Mouse 8000
> >>>
> >>> Attached is patch in the proper format.
> >>
> >> I see the need for this one, but just a failing UTF-8 check to  
> >> assume we
> >> are using ISO-8859-1 is not good enough. Since the mouse has a DID
> >> record, can we tie this together with the VID and PID?
> >
> > In those kind of cases, we can only guess what the encoding would  
> > be, we
> > can't detect those encodings reliably. Given that we fallback to the  
> > old
> > behaviour when the device, and that ISO-8859-X corresponds 1-1 with
> > UTF-8 for the ASCII characters, I don't think it's such a stretch to
> > think that American companies (who'd be using ISO-8859-1) would be the
> > worst offenders for this sort of mistake.
> >
> > I don't think special casing only this device would be a good idea.  
> > As I
> > mentioned, I reproduced the bug by making my computer, running BlueZ,
> > adopt that name.
> 
> can we just fallback to ASCII only? Looks better to me than assuming  
> Latin-1. We replace invalid characters with spaces in that case.

Patch to do that attached.

Cheers

[-- Attachment #2: 0001-Handle-non-UTF-8-device-names.patch --]
[-- Type: text/x-patch, Size: 1227 bytes --]

>From 02180fd9a53d3e544a218b3040ce63dcb58b4560 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 8 Nov 2010 16:43:42 +0000
Subject: [PATCH] Handle non-UTF-8 device names

http://thread.gmane.org/gmane.linux.bluez.kernel/1687
https://bugzilla.redhat.com/show_bug.cgi?id=450081
---
 src/event.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/event.c b/src/event.c
index d5bf967..0a2fb0c 100644
--- a/src/event.c
+++ b/src/event.c
@@ -562,13 +562,15 @@ void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	struct remote_dev_info match, *dev_info;
 
 	if (status == 0) {
-		char *end;
-
-		/* It's ok to cast end between const and non-const since
-		 * we know it points to inside of name which is non-const */
-		if (!g_utf8_validate(name, -1, (const char **) &end))
-			*end = '\0';
-
+		if (!g_utf8_validate(name, -1, NULL)) {
+			/* Assume ASCII, and replace all non-ASCII with spaces */
+			for (i = 0; name[i] != '\0'; i++) {
+				if (!isascii (name[i]))
+					name[i] = ' ';
+			}
+			/* Remove leading and trailing whitespace characters */
+			g_strstrip (name);
+		}
 		write_device_name(local, peer, name);
 	}
 
-- 
1.7.5

Re: [PATCH] Re: Work-around for broken MS device

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On Thu, 2011-05-05 at 19:02 +0100, Bastien Nocera wrote:
> On Mon, 2009-03-23 at 16:45 +0100, Marcel Holtmann wrote:
> > Hi Bastien,
> > 
> > >>>> As seen in https://bugzilla.redhat.com/show_bug.cgi?id=450081
> > >>>> The Microsoft Wireless Notebook Presenter Mouse 8000 has its name  
> > >>>> in
> > >>>> ISO-8859-1 instead of UTF-8, as required by the BT spec.
> > >>>>
> > >>>> I've implemented a small work-around. This isn't very invasive,  
> > >>>> IMO, as
> > >>>> we already do UTF-8 checks.
> > >>>>
> > >>>> In my tests, this makes the mouse show up as:
> > >>>> Microsoft® Wireless Notebook Presenter Mouse 8000
> > >>>
> > >>> Attached is patch in the proper format.
> > >>
> > >> I see the need for this one, but just a failing UTF-8 check to  
> > >> assume we
> > >> are using ISO-8859-1 is not good enough. Since the mouse has a DID
> > >> record, can we tie this together with the VID and PID?
> > >
> > > In those kind of cases, we can only guess what the encoding would  
> > > be, we
> > > can't detect those encodings reliably. Given that we fallback to the  
> > > old
> > > behaviour when the device, and that ISO-8859-X corresponds 1-1 with
> > > UTF-8 for the ASCII characters, I don't think it's such a stretch to
> > > think that American companies (who'd be using ISO-8859-1) would be the
> > > worst offenders for this sort of mistake.
> > >
> > > I don't think special casing only this device would be a good idea.  
> > > As I
> > > mentioned, I reproduced the bug by making my computer, running BlueZ,
> > > adopt that name.
> > 
> > can we just fallback to ASCII only? Looks better to me than assuming  
> > Latin-1. We replace invalid characters with spaces in that case.
> 
> Patch to do that attached.

And one that compiles (sigh).

[-- Attachment #2: 0001-Handle-non-UTF-8-device-names.patch --]
[-- Type: text/x-patch, Size: 1389 bytes --]

>From 2b4524b060d980a7899c03359cd4943c9ad9cfe4 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 8 Nov 2010 16:43:42 +0000
Subject: [PATCH] Handle non-UTF-8 device names

http://thread.gmane.org/gmane.linux.bluez.kernel/1687
https://bugzilla.redhat.com/show_bug.cgi?id=450081
---
 src/event.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/event.c b/src/event.c
index d5bf967..6805c0b 100644
--- a/src/event.c
+++ b/src/event.c
@@ -28,6 +28,7 @@
 
 #define _GNU_SOURCE
 #include <stdio.h>
+#include <ctype.h>
 #include <errno.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -562,13 +563,17 @@ void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	struct remote_dev_info match, *dev_info;
 
 	if (status == 0) {
-		char *end;
-
-		/* It's ok to cast end between const and non-const since
-		 * we know it points to inside of name which is non-const */
-		if (!g_utf8_validate(name, -1, (const char **) &end))
-			*end = '\0';
-
+		if (!g_utf8_validate(name, -1, NULL)) {
+			guint i;
+
+			/* Assume ASCII, and replace all non-ASCII with spaces */
+			for (i = 0; name[i] != '\0'; i++) {
+				if (!isascii (name[i]))
+					name[i] = ' ';
+			}
+			/* Remove leading and trailing whitespace characters */
+			g_strstrip (name);
+		}
 		write_device_name(local, peer, name);
 	}
 
-- 
1.7.5

Re: [PATCH] Re: Work-around for broken MS device

[Johan Hedberg]

Hi Bastien,

On Thu, May 05, 2011, Bastien Nocera wrote:
> > Patch to do that attached.
> 
> And one that compiles (sigh).

> From 2b4524b060d980a7899c03359cd4943c9ad9cfe4 Mon Sep 17 00:00:00 2001
> From: Bastien Nocera <hadess@hadess.net>
> Date: Mon, 8 Nov 2010 16:43:42 +0000
> Subject: [PATCH] Handle non-UTF-8 device names
> 
> http://thread.gmane.org/gmane.linux.bluez.kernel/1687
> https://bugzilla.redhat.com/show_bug.cgi?id=450081
> ---
>  src/event.c |   19 ++++++++++++-------
>  1 files changed, 12 insertions(+), 7 deletions(-)

Pushed upstream. Thanks.

Johan