New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added option to setgid, same as setuid #3

Merged
merged 1 commit into from Jan 25, 2015

Conversation

Projects
None yet
2 participants
@rvandam
Contributor

rvandam commented Jan 25, 2015

Adding the code to support setgid was pretty trivial. However, finding a portable way to test that functionality is not trivial. As it was, the exisitng setuid feature was untested so I had hoped to add a test for both features.

I considered using the nobody/nogroup pair but that creates two problems. The first problem is that the 'nobody' user likely doesn't exist on platforms such as Win32. The second problem is that the 'nobody' user very likely can't write to the t/ test directory (as the other tests do to allow communication between the spawned daemons). I could use File::Spec->tmpdir to get a more portable temp path that is likely world writable but that still doesn't guarantee the 'nobody' user will exist or will be able to write to that path.

In the end, the only test I was reasonably sure would work cross platform was to just setgid and setuid back to the current effective group/user. That is basically a noop but at least it provides a tiny improvement in test coverage :(.

Let me know if you can think of a better way to test this.

@akreal

This comment has been minimized.

Show comment
Hide comment
@akreal

akreal Jan 25, 2015

Owner

I can't think of any way to test this, too.

Owner

akreal commented Jan 25, 2015

I can't think of any way to test this, too.

akreal added a commit that referenced this pull request Jan 25, 2015

Merge pull request #3 from rvandam/setgid
added option to setgid, same as setuid

@akreal akreal merged commit 780a855 into akreal:master Jan 25, 2015

@akreal

This comment has been minimized.

Show comment
Hide comment
@akreal

akreal Jan 25, 2015

Owner

Big thanks for your effort!

Owner

akreal commented Jan 25, 2015

Big thanks for your effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment