Bug 5198 - Samba Fails to Split GECOS on Comma
Samba Fails to Split GECOS on Comma
Status: RESOLVED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: User/Group Accounts
3.0.26a
x64 Linux
: P3 normal
: none
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-14 10:52 UTC by Richard Laager
Modified: 2010-03-02 06:23 UTC (History)
3 users (show)

See Also:
jra: review+


Attachments
Alternative patch (2.10 KB, patch)
2010-02-06 06:28 UTC, Volker Lendecke
no flags Details
patch (1.54 KB, patch)
2010-02-09 23:42 UTC, Jesse Malone
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Laager 2008-01-14 10:52:29 UTC
Pulling up the Start Menu on a Windows XP machine joined to a Samba PDC
shows, for example, "Richard Laager,,," as the full name. This is
because the GECOS field on the server contains commas, apparently added
by chfn(1). Samba should stop at the first comma.

See also: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=460494
Comment 1 Lou Goddard 2008-03-21 14:56:12 UTC
I see the same issue Richard Laager reported.

Red Hat Enterprise Linux ES release 3 (Taroon Update 9)
samba-3.0.9-1.3E.10

Comment 2 Richard Laager 2008-12-12 13:07:28 UTC
A comment here might be useful for implementing this:
https://bugs.freedesktop.org/show_bug.cgi?id=18699#c5
Comment 3 Jesse Malone 2010-02-05 11:45:35 UTC
I can confirm this in samba version 3.4.3

I'd really like to get this resolved. I don't know the internals of samba too well. Any thoughts on parsing gecos in pdb_get_fullname() (pdb_get_set.c)?

(In reply to comment #2)
> A comment here might be useful for implementing this:
> https://bugs.freedesktop.org/show_bug.cgi?id=18699#c5
> 

Comment 4 Jesse Malone 2010-02-05 12:49:30 UTC
I'll start off by saying I really don't know this project so opinions/corrections/suggestions are very much appreciated. Proposed fix (untested thus far):

For samba 3.4.3 in passdb/pdb_get_set.c, at line 36

const char *pdb_fullname_from_gecos(const char* gecos) {
        // the sampass->fullname field doesn't separate
        // fullname from the other gecos fields. This
        // will grab just the name
         size_t len;
         char* fullname;
         len = strcspn(gecos,",");
         fullname = malloc((len+1)*sizeof(char));
         strncpy(fullname,gecos,len);
         fullname[len] = '\0'; //ensure null termination
         return fullname;
}
const char *pdb_get_fullname(const struct samu *sampass)
{
        return pdb_fullname_from_gecos(sampass->full_name);
}


(In reply to comment #3)
> I can confirm this in samba version 3.4.3
> 
> I'd really like to get this resolved. I don't know the internals of samba too
> well. Any thoughts on parsing gecos in pdb_get_fullname() (pdb_get_set.c)?
> 
> (In reply to comment #2)
> > A comment here might be useful for implementing this:
> > https://bugs.freedesktop.org/show_bug.cgi?id=18699#c5
> > 
> 

Comment 5 Volker Lendecke 2010-02-06 06:28:53 UTC
Created attachment 5292 [details]
Alternative patch

Can you test the attached patch? Should apply cleanly to 3.4.

Volker
Comment 6 Jesse Malone 2010-02-09 23:42:02 UTC
Created attachment 5311 [details]
patch

Looking good now. count_commas was looping indefinitely due to a wee typo. Here's the working patch.

Thanks
Comment 7 Volker Lendecke 2010-02-13 10:27:16 UTC
Thanks for testing the patch!

I've pushed it to master with 2ea2d2a. I don't want to load the 3.5.0 release with yet another patch, so it will probably go into 3.5.1 and one of the next 3.4 releases.

Jeremy, please check this and if ok ack it for 3.5.1/3.4.x

Thanks,

Volker
Comment 8 Volker Lendecke 2010-02-13 10:57:34 UTC
Try to get the review flags right...
Comment 9 Jeremy Allison 2010-02-22 16:00:16 UTC
+1 patch looks good for 3.5.1. Re-assigning to Karolin for inclusion in 3.5.1.
Jeremy.
Comment 10 Karolin Seeger 2010-03-02 06:23:35 UTC
Pushed to v3-5-test and v3-4-test.
Closing out bug report.

Thanks!