Bug 174352 - finger munges UTF-8 GECOS information
Summary: finger munges UTF-8 GECOS information
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: finger
Version: 4.0
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
: ---
Assignee: Radek Vokál
QA Contact: Mike McLean
URL:
Whiteboard:
Depends On:
Blocks: 187538
TreeView+ depends on / blocked
 
Reported: 2005-11-28 12:08 UTC by Bastien Nocera
Modified: 2014-09-04 11:01 UTC (History)
2 users (show)

Fixed In Version: RHBA-2006-0342
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 490443 (view as bug list)
Environment:
Last Closed: 2006-05-17 19:34:32 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
finger-munge-utf8-gecos.patch (505 bytes, patch)
2005-11-28 12:08 UTC, Bastien Nocera
no flags Details | Diff
finger-i18n.sh (519 bytes, text/plain)
2005-11-28 12:09 UTC, Bastien Nocera
no flags Details
bsd-finger-wide-char-support.patch (2.57 KB, patch)
2005-12-12 12:12 UTC, Bastien Nocera
no flags Details | Diff
bsd-finger-wide-char-support2.patch (2.72 KB, patch)
2005-12-12 12:57 UTC, Bastien Nocera
no flags Details | Diff
bsd-finger-wide-char-support3.patch (2.84 KB, patch)
2005-12-12 13:44 UTC, Bastien Nocera
no flags Details | Diff
bsd-finger-wide-char-support4.patch (4.02 KB, patch)
2005-12-13 15:38 UTC, Bastien Nocera
no flags Details | Diff
use fprintf for whole UTF8 line (3.73 KB, patch)
2005-12-15 09:09 UTC, Radek Vokál
no flags Details | Diff
bsd-finger-wide-char-support6.patch (3.75 KB, patch)
2005-12-15 14:21 UTC, Radek Vokál
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2006:0342 0 normal SHIPPED_LIVE finger bug fix update 2006-05-17 04:00:00 UTC

Description Bastien Nocera 2005-11-28 12:08:20 UTC
finger-0.17-26

When a UTF-8 real username is created, fingering that user will print out
garbage in place of the username, as finger implements "fputs()" using fputc,
which doesn't understand wide characters.

Patch and testcase attached below.

Comment 1 Bastien Nocera 2005-11-28 12:08:21 UTC
Created attachment 121538 [details]
finger-munge-utf8-gecos.patch

Comment 2 Bastien Nocera 2005-11-28 12:09:18 UTC
Created attachment 121539 [details]
finger-i18n.sh

Comment 3 Radek Vokál 2005-11-28 13:09:34 UTC
Thanks, patched applied on rawhide

Comment 6 Bastien Nocera 2005-12-07 08:43:25 UTC
Radek, this patch doesn't take into account:
- 7-bit terminals
- the send_crs option
- stripping non-printable characters

I will fix those up, and upload a corrected patch soon.

Comment 7 Radek Vokál 2005-12-08 10:58:02 UTC
Great, how about using Octalify makro from file to make nonprintable characters
displayed?

Comment 8 Bastien Nocera 2005-12-08 18:13:44 UTC
Yep, would still need to take into account the 2 other ones. But doing a putc on
each character is definitely a mistake.
I'll check it out tomorrow, if I have some spare time.

Comment 9 Bastien Nocera 2005-12-12 12:12:06 UTC
Created attachment 122127 [details]
bsd-finger-wide-char-support.patch

A patch that does all that (hopefully). No autotool-fu, there's no configure.in
or .ac in the tarball...

Comment 10 Radek Vokál 2005-12-12 12:48:05 UTC
I'm not sure it works now :) At least your own test script gives me broken output 

root@garfield finger# ./finger test-i18n
Login: test-i18n                        Name:
¥µ\uffffM-^E\uffffM-^E\u04b5\uffffM-^U\u4e25\u5f25\u6f25



Comment 11 Bastien Nocera 2005-12-12 12:49:58 UTC
Well, you will need to have all this defined for it to work:
+#if defined(HAVE_WCHAR_H) && defined(HAVE_MBRTOWC) && defined(HAVE_WCWIDTH)

As there's no configure-fu, hard-coding it at the top of display.c might be a
short-term work-around.

Comment 12 Bastien Nocera 2005-12-12 12:52:52 UTC
And it's quite broken still :)

Comment 13 Bastien Nocera 2005-12-12 12:57:17 UTC
Created attachment 122128 [details]
bsd-finger-wide-char-support2.patch

Comment 14 Radek Vokál 2005-12-12 13:04:27 UTC
Ehm, again, here's my output

# ./finger test-i18n
Login: test-i18n                        Name:
¥µ\uffffM-^E\uffffM-^E\u04b5\uffffM-^U\u4e25\u5f25\u6f25
finger: display.c:226: fxputs: Assertion `bytesconsumed != (size_t)(-1) &&
bytesconsumed != (size_t)(-2)' failed.
Directory: /home/test-i18n          Aborted

and reading your comment in the patch :)
/* This isn't supposed to happen as we verified the string before hand */



Comment 15 Bastien Nocera 2005-12-12 13:19:11 UTC
Yeah, I'm currently testing with hard-coded defines. Don't understand why it
doesn't parse the username as a valid multi-byte string...

Comment 16 Bastien Nocera 2005-12-12 13:44:11 UTC
Created attachment 122135 [details]
bsd-finger-wide-char-support3.patch

This *should* work, but for some reason, it doesn't parse the string properly,
and think it's not a valid multi-byte string. I have no idea why it doesn't
work...
Can you find my stupid typo somewhere there?

Comment 17 Radek Vokál 2005-12-13 08:09:11 UTC
Ok, I'll try to find it out today. From the first glance, I don't think that
verifymultibate function is correct. Debugger shows that it returns 1 at a very
first multibyte character it hits. Closely mbrtowc returns -1 when for ¥. 

Comment 18 Bastien Nocera 2005-12-13 09:27:06 UTC
yeah, which is why I think there's a typo somewhere there...

Comment 19 Radek Vokál 2005-12-13 09:44:54 UTC
Hmm, I thought this will correctly test whole line as multibyte string but it
also fails when a single multibyte character appears in the line :( So is there
sth wrong with the input buffer?

static int verifymultibyte(const char *buf) {
	mbstate_t state;
	wchar_t *wbuf;
	size_t bytesconsumed;
	(void)memset(&state, 0, sizeof(mbstate_t));
	int n = strlen(buf);
	(void)wmemset(wbuf, 0, 2*n);
	int x;
	if ((x = mbsrtowcs(wbuf, &buf,n,&state)) > 0) {
	    printf("Success (length=%x)\n",x);
	    return 1;
	} else {
	    printf("Fail\n");
	    return 0;
	}
}

Comment 20 Radek Vokál 2005-12-13 11:19:47 UTC
Created attachment 122173 [details]
4th revision of widechar patch

Ok, this works for me. Needs some more clean-up but it should be fine ...

Comment 22 Bastien Nocera 2005-12-13 15:38:08 UTC
Created attachment 122181 [details]
bsd-finger-wide-char-support4.patch

And a patch that actually works. Only difference with my -3 patch is the call
to setlocale(). Thanks Jakub for pointing that out.

Comment 23 Radek Vokál 2005-12-14 09:48:45 UTC
Yep, works good now. Applied on rawhide.

Comment 25 Radek Vokál 2005-12-15 09:09:36 UTC
Created attachment 122267 [details]
use fprintf for whole UTF8 line

Here's another patch. The change is, that converted line is printed as a whole
and not only char by char. Should be really fine now.

Comment 31 Bastien Nocera 2005-12-15 10:27:26 UTC
Radek, the updated patch doesn't take into account the send_crs option, or strip
non-printable characters if the string is a proper multi-byte one.

Comment 32 Radek Vokál 2005-12-15 10:30:29 UTC
Those should be converted by OCTALIFY, aren't they?

Comment 33 Bastien Nocera 2005-12-15 10:34:59 UTC
+			} else if (bytesconsumed == 1) {
+				op++;
+			} else {
This should be special casing stuff like \n

+				char *tmp;
+				tmp = buffer;
+				buffer[bytesconsumed] = '\0';
+				while (bytesconsumed-- > 0) {
+					OCTALIFY(tmp, op);
+				}

tmp (ie. buffer) is used to store the OCTALIFY output, but isn't used afterwards.

Comment 34 Radek Vokál 2005-12-15 12:32:10 UTC
IHMO \n will be processed by the if above, cos it will have only one byte. Maybe
this if should have also isprint test for one byte characters. 

   } else if (bytesconsumed == 1 && isprint(op[0])) {

and the rest will be skipped or converted. 

Comment 35 Radek Vokál 2005-12-15 13:14:46 UTC
of course 'isprint(op[eop-op])'

Comment 36 Radek Vokál 2005-12-15 13:47:13 UTC
ok, I got bit lost in this UTF-8 thing. Why do we want to strip \n and others?
If I look at the original output, it contains newline characters .. is this
wrong? I would like to see some clear example of some breakage it can cause. 

Also what does this sentence mean - "... or strip non-printable characters if
the string is a proper multi-byte one" - don't see where I lost these ones. 


The above comment is wrong, op is already shifted. 

Comment 37 Radek Vokál 2005-12-15 14:21:47 UTC
Created attachment 122281 [details]
bsd-finger-wide-char-support6.patch

this patch respects send_crs options. Still remains the problem with some
non-printable characters.

Comment 44 Red Hat Bugzilla 2006-05-17 19:34:32 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2006-0342.html



Note You need to log in before you can comment on or make changes to this bug.