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

Handle empty username #16823

Merged
merged 1 commit into from Feb 23, 2018

Conversation

Projects
None yet
3 participants
@Arcanemagus
Contributor

Arcanemagus commented Feb 22, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Changes Atom to use os.userInfo().username to determine the user name, defaulting to an empty string if it still can't be determined.

Alternate Designs

If the username isn't necessarily required for the uniqueness of the hash, a simpler and probably slightly faster method would be to keep the current method of determining it and simply default to the empty string.

Why Should This Be In Core?

This is fixing a bug in core.

Benefits

Atom doesn't crash when USERNAME/USER isn't defined.

Possible Drawbacks

This is likely to be slightly slower than the previous method of just checking environment variables.

Verification Process

I verified this was an issue using the steps outlined in #16821. To verify that this fixes the issue I set up an environment that the current version (v1.25.0-beta2) fails in, built a version based on this PR, and verified that the new build works properly in that environment.

Applicable Issues

Fixes #16821.

🐛 Handle empty username
If Atom is launched in a shell that is missing the `USER`/`USERNAME`
environment variable it locks up due to an unhandled error from
`crypto.update`. This switches to using Node.js's built in
`os.userInfo()` method to retrieve the current user name,
allowing Atom to complete initialization in this scenario.

Fixes #16821.

@Arcanemagus Arcanemagus changed the title from :bug: Handle empty username to Handle empty username Feb 22, 2018

@daviwil

Looks good to me!

@maxbrunsfeld maxbrunsfeld merged commit 4524763 into atom:master Feb 23, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Feb 23, 2018

Thanks @Arcanemagus!

maxbrunsfeld added a commit that referenced this pull request Feb 23, 2018

@Arcanemagus Arcanemagus deleted the Arcanemagus:allow-empty-username branch Feb 23, 2018

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