Uh oh!
There was an error while loading. Please reload this page.
Return empty string instead of throwing when getpwuid_r fails unexpectedly - #132396
Return empty string instead of throwing when getpwuid_r fails unexpectedly#132396HarnageaGabriel wants to merge 2 commits into
Conversation
…tedly GetUserNameFromPasswd's doc contract already states it returns an empty string on failure, matching the existing behavior when the current user has no passwd entry. But any other getpwuid_r error (e.g. ENOENT when /etc/passwd is missing, as in restricted sandboxes/containers) fell through to an IOException, crashing callers like Environment.UserName and, transitively, dotnet build/dotnet new. Fixesdotnet#119216
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
HarnageaGabriel
commented
Aug 17, 2026
@dotnet-policy-service agree |
Tagging subscribers to this area: @dotnet/area-system-runtime |
| throw new IOException(errorInfo.GetErrorMessage(), errorInfo.RawErrno); | ||
| // Otherwise, give back null. | ||
| username = null; | ||
| return true; |
There was a problem hiding this comment.
This seems like a user visible behavioral change that should have a test. Clearly this was a test hole when it was written and now that we're addressing it here we might consider taking the opportunity to add some verification?
/cc @jeffhandley
There was a problem hiding this comment.
Good call — added coverage in ee86dae.
I extracted the error-code decision out of TryGetUserNameFromPasswd into a small internal helper, ShouldRetryGetUserNameFromPasswd(int error), which only decides "retry with a larger buffer" (true, ERANGE) vs. "no user name" (false, everything else including -1/not-found and unexpected errors). The native getpwuid_r call and all other logic are unchanged.
Since a real "unexpected getpwuid_r failure" (e.g. EIO) isn't something you can reliably force through the actual syscall in a portable xunit test, and there's no existing DI seam for this P/Invoke shim, I unit-tested the extracted helper directly instead — same pattern already used by Win32Marshal.Tests in this test project for similar error-code-to-outcome logic. New tests in GetPwUidTests.cs cover:
-1(user not found) → no retryERANGE→ retryEIO(stand-in for an unexpected error) → no retry, which is what now surfaces asEnvironment.UserNamereturning""instead of throwing.
I wasn't able to run the full test suite locally (this repo's local build requires enabling Windows long paths system-wide, which I didn't want to do unilaterally in this environment), but the change is a pure extraction with no behavioral difference, and I reviewed the resulting logic carefully against the original branching.
Extract the error-code decision in TryGetUserNameFromPasswd into an internal ShouldRetryGetUserNameFromPasswd helper so it can be unit tested independent of the native getpwuid_r call. Adds tests covering the not-found (-1), ERANGE (retry), and unexpected-error (e.g. EIO) cases, addressing review feedback that the throw-to-empty-string behavior change was untested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes#119216
Problem
GetUserNameFromPasswd's XML doc already documents that it returns an empty string on failure. That contract already holds when/etc/passwdis readable but the current UID isn't present (getpwuid_rreturns-1/ENOENT-equivalent →TryGetUserNameFromPasswdreturnsnull).But when
getpwuid_rfails for any other reason — notably when/etc/passwditself doesn't exist, as can happen in restricted sandboxes/containers —TryGetUserNameFromPasswdthrew anIOExceptioninstead. That exception propagates up throughEnvironment.UserNameand crashes the calling process (e.g.dotnet build,dotnet new) instead of degrading gracefully.Repro (from the issue)
Fix
src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetPwUid.cs: inTryGetUserNameFromPasswd, the fallback branch after theERANGE(buffer-too-small) check no longer throws — it now setsusername = nulland returnstrue, same as the "current user not found" path, soGetUserNameFromPasswdreturnsstring.Emptyinstead of propagating the exception. TheERANGEretry-with-larger-buffer behavior is unchanged.Also removed the now-unused
using System.IO;.Testing
This is a small interop fix in
src/libraries/Common(shared source, no dedicated isolated test project); given the size of the dotnet/runtime repo I did not run a full runtime build. I verified the change by inspecting the diff and confirming no other caller relies on the previously-thrownIOException(checkedEnvironment.UserNameinEnvironment.Unix.cs, the only caller).