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

cmd/utils/flags.go: Applying a String len guard for the gitCommit param of the NewApp() #15790

Merged
merged 2 commits into from
Jan 3, 2018

Conversation

epappas
Copy link
Contributor

@epappas epappas commented Jan 2, 2018

Just a simple guard to match the check-case of [1]. Seems that for some applications, people are trying to sneak in the git-tag (version) of theirs, rather than an actual commit hash. This is only to protect them.

[1] https://github.com/ethereum/go-ethereum/blob/master/params/version.go#L41

@GitCop
Copy link

GitCop commented Jan 2, 2018

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: f3d77506ad8491b2fca3295f88bdb94498fff92f
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@epappas epappas changed the title Applying a String len guard for git commits for the NewApp() cmd/utils/flags.go: Applying a String len guard for git commits for the NewApp() Jan 2, 2018
@epappas epappas force-pushed the fix/string-len-guard branch from f3d7750 to 89be5f7 Compare January 2, 2018 09:44
@epappas epappas changed the title cmd/utils/flags.go: Applying a String len guard for git commits for the NewApp() cmd/utils/flags.go: Applying a String len guard for the gitCommit param of the NewApp() Jan 2, 2018
@@ -96,7 +96,7 @@ func NewApp(gitCommit, usage string) *cli.App {
//app.Authors = nil
app.Email = ""
app.Version = params.Version
if gitCommit != "" {
if gitCommit != "" && len(gitCommit) >= 8 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the gitCommit != "" part, len(gitCommit) >= 8 already validates that it's not "".

@karalabe karalabe added this to the 1.8.0 milestone Jan 3, 2018
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@karalabe karalabe merged commit 7a59a93 into ethereum:master Jan 3, 2018
cooganb referenced this pull request in cooganb/go-ethereum Jan 23, 2018
…#15790)

* cmd/utils/flags.go: Applying a String len guard for the gitCommit param of the NewApp()

* cmd/utils: remove redundant clause in if condition
cooganb referenced this pull request in cooganb/go-ethereum Feb 23, 2018
…#15790)

* cmd/utils/flags.go: Applying a String len guard for the gitCommit param of the NewApp()

* cmd/utils: remove redundant clause in if condition
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
…thereum#15790)

* cmd/utils/flags.go: Applying a String len guard for the gitCommit param of the NewApp()

* cmd/utils: remove redundant clause in if condition
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.

3 participants