libcontainer: user: always treat numeric ids numerically#708
Conversation
|
@tianon I know it's been a while, but I've refactored most of the code you wrote a few years ago. Want to take a look? :P |
| // panic, because this is a programming/logic error, not a runtime one | ||
| panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!") | ||
| // Someone goof'd when writing code using this function. Scream so they can hear us. | ||
| panic(fmt.Sprintf("parseLine only accepts {*string, *int, *[]string} as arguments! %#v\n is not a pointer!", e)) |
There was a problem hiding this comment.
lets not do a new line here ?
|
@dqminh Nits addressed. 😸 |
| } | ||
| return u.Name == userArg || strconv.Itoa(u.Uid) == userArg | ||
|
|
||
| if uid, err := strconv.Atoi(userArg); err == nil { |
There was a problem hiding this comment.
Do you think it'd be worthwhile or possible to do this test/conversion once instead of once per passwd entry like it's doing now, plus one more time outside this filter if we don't find the UID in the file?
There was a problem hiding this comment.
You're right, I didn't consider the fact that it doesn't shortcircuit anymore. But since it's a constant for the entire function we can definitely refactor our essentially all calls to Atoi. I'm just not sure what to do about negative UIDs.
|
Other than the fact that we're hitting |
|
Thanks @cyphar ! |
Most shadow-related tools don't treat numeric ids as potential usernames, so change our behaviour to match that. Previously, using an explicit specification like 111:222 could result in the UID and GID not being 111 and 222 respectively (which is confusing). Signed-off-by: Aleksa Sarai <[email protected]>
|
@tianon I've addressed your concern about calling |
Signed-off-by: Aleksa Sarai <[email protected]>
Some of the code was quite confusing inside libcontainer/user, so refactor and comment it so future maintainers can understand what's going and what edge cases we have to deal with. Signed-off-by: Aleksa Sarai <[email protected]>
|
awesome, thanks all |
|
🤘 |
… as uid numbers Upstream reference: opencontainers/runc#708 Signed-off-by: Mrunal Patel <[email protected]> Signed-off-by: Antonio Murdaca <[email protected]>
… as uid numbers Upstream reference: opencontainers/runc#708 Signed-off-by: Mrunal Patel <[email protected]> Signed-off-by: Antonio Murdaca <[email protected]>
config-solaris: Fix "VNIC`s" -> "VNIC's" typo
6 participants
Most shadow-related tools don't treat numeric ids as potential
usernames, so change our behaviour to match that. Previously, using an
explicit specification like 111:222 could result in the UID and GID not
being 111 and 222 respectively (which is confusing).
Fixes #695
Signed-off-by: Aleksa Sarai [email protected]