[Mono-dev] [PATCH] Bug in Mono.Posix handling '+::::::' entry in /etc/passwd

[Jonathan Pryor]

So, in short: your /etc/passwd database is corrupt, and instead of
fixing the database you want to "fix" UnixUserInfo to ignore "invalid"
users, where "invalid" users are those with a name of "+".  (And why
just "+", as opposed to any larger set of strings?  Because that makes
things work with your corrupt database, that's why...)

What the UnixUserTest.NonReentrantSyscalls() test checks for is the
following: for every user within the /etc/passwd database (where "user"
is defined as "whatever is returned by getpwent(3)"), can we get the
"same" user back when looking up by the user name and by the user id
fields?

In short, it assumes that /etc/passwd is _sane_, then makes sure that if
there's an entry within the database, we can actually lookup the entry
by name/id, as would be expected by most people.

Furthermore, the one reference I could ~quickly find regarding the
constraints on usernames is [0], which says:

        IEEE Std 1003.1-2001 is silent about the content of the strings
        containing user or group names. These could be digit strings.

Which to me says that we can't categorically say that '+' will always be
an invalid user name, and thus it SHOULD NOT be skipped within
UnixUserInfo.GetLocalUsers().

The "best" I'd be willing to do is change the
UnixUserTest.NonReentrantSyscalls() tests to skip usernames with '+',
but I'd really rather not (as, again, there shouldn't be any reason to).

So why do you have '+::::::' in your /etc/passwd to begin with?  That's
not a valid entry anyway (no user id)!

 - Jon

[0]
http://www.opengroup.org/onlinepubs/000095399/xrat/xbd_chap03.html#tag_01_03_00_69

On Mon, 2009-04-27 at 05:11 -0700, Ian Dichkovsky wrote:
> Hello,
> 
> While running class library tests in our Mono MIPS 64-bit port we have met a
> failed test in Mono.Posix.
> Here is stacktrace
> 
> ============================================================================
> sstetskovych at sca-m0n8 ~/mono_new/mcs/class/Mono.Posix $ make check
> ok=:; make run-test-recursive || ok=false; make run-test-local || ok=false;
> $ok
> make[1]: Entering directory
> `/.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix'
> make[1]: Leaving directory
> `/.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix'
> make[1]: Entering directory
> `/.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix'
> MCS     [net_2_0] Mono.Posix_test_net_2_0.dll
> ok=:; \
>        MONO_REGISTRY_PATH="/home/sstetskovych/.mono/registry"
> MONO_PATH="./../../class/lib/net_2_0::$MONO_PATH"
> /home/sstetskovych/mono_new/mono/runtime/mono-wrapper --debug 
> ./../../class/lib/net_2_0/nunit-console.exe  Mono.Posix_test_net_2_0.dll
> -noshadow   -exclude=NotWorking,ValueAdd,CAS,InetAccess
> -output=TestResult-net_2_0.log -xml=TestResult-net_2_0.xml  || ok=false; \
>        (echo ''; cat TestResult-net_2_0.log) | sed '1,/^Tests run: /d';
> xsltproc ./../../build/nunit-summary.xsl TestResult-net_2_0.xml >>
> TestResult-net_2_0.log ; $ok
> NUnit version 2.4.8
> Copyright (C) 2002-2007 Charlie Poole.
> Copyright (C) 2002-2004 James W. Newkirk, Michael C. Two, Alexei A.
> Vorontsov.
> Copyright (C) 2000-2002 Philip Craig.
> All Rights Reserved.
> 
> Runtime Environment -
>   OS Version: Unix 2.6.18.8
>  CLR Version: 2.0.50727.1433 ( Mono 2.5 )
> 
> Excluded categories: NotWorking,ValueAdd,CAS,InetAccess
> ............................................................................................................................F.....................................
> Tests run: 161, Failures: 1, Not run: 0, Time: 49.421 seconds
> 
> Test Case Failures:
> 1) MonoTests.Mono.Unix.UnixUserTest.NonReentrantSyscalls : #TNRS: Exception
> constructing UnixUserInfo (string): NUnit.Framework.AssertionException:  
> #TNRS: access by name
>  Expected: not null
>  But was:  null
> 
>  at NUnit.Framework.Assert.That (System.Object actual,
> NUnit.Framework.Constraints.Constraint constraint, System.String message,
> System.Object[] args) [0x0002c] in
> /.root0/local/home/sstetskovych/mono_new/mcs/nunit24/NUnitFramework/framework/Assert.cs:2207
>  at NUnit.Framework.Assert.IsNotNull (System.Object anObject, System.String
> message, System.Object[] args) [0x00000] in
> /.root0/local/home/sstetskovych/mono_new/mcs/nunit24/NUnitFramework/framework/Assert.cs:171
>  at NUnit.Framework.Assert.IsNotNull (System.Object anObject, System.String
> message) [0x00000]
>  at MonoTests.Mono.Unix.UnixUserTest.NonReentrantSyscalls () [0x00032] in
> /.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix/Test/Mono.Unix/UnixUserTest.cs:91
> at MonoTests.Mono.Unix.UnixUserTest.NonReentrantSyscalls () [0x00083] in
> /.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix/Test/Mono.Unix/UnixUserTest.cs:99
> at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke
> (object,object[],System.Exception&)
> at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags
> invokeAttr, System.Reflection.Binder binder, System.Object[] parameters,
> System.Globalization.CultureInfo culture) [0x000ca] in
> /.root0/local/home/sstetskovych/mono_new/mcs/class/corlib/System.Reflection/MonoMethod.cs:169
> 
> 
> make[1]: *** [run-test-lib] Error 1
> make[1]: Leaving directory
> `/.root0/local/home/sstetskovych/mono_new/mcs/class/Mono.Posix'
> make: *** [do-run-test] Error 1
> ============================================================================
> 
> 
> Our research showed that function 'public static UnixUserInfo[]
> GetLocalUsers ()' in mcs/class/Mono.Posix/Mono.Unix/UnixUserInfo.cs
> encounters '+::::::' entry in /etc/passwd and then fills corresponding
> members in UnixInfoStructure with nulls.
> Test 'public void NonReentrantSyscalls ()' fail on assert 'Assert.IsNotNull
> (byName, "#TNRS: access by name");'
> We have created a patch which fixes this test
> 
> ===================================================================
> --- class/Mono.Posix/Mono.Unix/UnixUserInfo.cs  (revision 166)
> +++ class/Mono.Posix/Mono.Unix/UnixUserInfo.cs  (working copy)
> @@ -176,8 +176,10 @@
>                                }
>                                try {
>                                        Native.Passwd p;
> -                                       while ((p =
> Native.Syscall.getpwent()) != null)
> -                                               entries.Add (new
> UnixUserInfo (p));
> +                                        while ((p =
> Native.Syscall.getpwent()) != null){
> +                                               if (p.pw_name != "+")
> +                                                       entries.Add (new
> UnixUserInfo (p));
> +                                       }
>                                        if (Native.Syscall.GetLastError () !=
> (Native.Errno) 0)
>                                               
> UnixMarshal.ThrowExceptionForLastError ();
>                                } 
> ===================================================================
> 
> Probably, we have missed something in logic of tests or implementation of
> functions. So, it would be good,  if you could explain what we have missed.
> 
> Regards,
> Ian