Skip to content
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

Adjust parser ordering to error out on invalid names #26

Closed
wants to merge 1 commit into from

Conversation

tianon
Copy link
Contributor

@tianon tianon commented Feb 26, 2019

See tianon/gosu@fd60171 and tianon/gosu@f87df69.

The basic problem is that an invalid name like bogus will quietly run as root instead of being denied entirely (which those test updates look for and this change is verified to fix).

I've run this updated implementation successfully against all of the test cases in https://github.com/tianon/gosu/blob/f87df69c868e19f7258b4facb7c2472d76d98dda/Dockerfile.test.

(See also tianon/gosu#57 (comment) / tianon/gosu#60.)

Closes #6

See tianon/gosu@fd60171 and tianon/gosu@f87df69.

The basic problem is that an invalid name like `bogus` will quietly run as `root` instead of being denied entirely (which those test updates look for and this change is verified to fix).

I've run this updated implementation successfully against all of the test cases in https://github.com/tianon/gosu/blob/f87df69c868e19f7258b4facb7c2472d76d98dda/Dockerfile.test.
@ken0x0a
Copy link

ken0x0a commented Jun 7, 2019

@ncopa Could you merge this, please?

@tianon
Copy link
Contributor Author

tianon commented Sep 18, 2019

Friendly ping @ncopa 🙏 ❤️

Anything about this you'd like me to change/update?

@ncopa
Copy link
Owner

ncopa commented Sep 18, 2019

sorry, i completely missed this.

@ncopa
Copy link
Owner

ncopa commented Sep 18, 2019

fixed with 212b751

@ncopa ncopa closed this Sep 18, 2019
@tianon tianon deleted the edge-cases branch September 18, 2019 20:06
@tianon
Copy link
Contributor Author

tianon commented Feb 9, 2021

Any chance of a 0.3 with this change in it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exit code should be non-zero for inexistent user
3 participants