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

Don't call open("") backport #1362

Merged
merged 1 commit into from Apr 22, 2022
Merged

Don't call open("") backport #1362

merged 1 commit into from Apr 22, 2022

Conversation

kolyshkin
Copy link
Contributor

What type of PR is this?

  • bug

What this PR does / why we need it:

strings.Split(s, sep) returns a slice of a single element containing s
if sep is not found in s. This is true even if s is empty.

As a result, every call to flagFromEnvOrFile results in an attempt to
open a file with empty name. This is seen from strace as

[pid 3287620] openat(AT_FDCWD, "", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
[pid 3287620] openat(AT_FDCWD, "", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
...

To fix, check if the string is empty before calling ReadFile.

This also fixes cases where filePath is non-empty but has extra commas.

Which issue(s) this PR fixes:

none

Special notes for your reviewer:

This is a backport of #1336 to v1 branch.

Testing

Due to the obvious nature of the fix, no additional testing is required.

Release Notes

Fix calling open(2) with empty file name.

strings.Split(s, sep) returns a slice of a single element containing s
if sep is not found in s. This is true even if s is empty.

As a result, every call to flagFromEnvOrFile results in an attempt to
open a file with empty name. This is seen from strace as

[pid 3287620] openat(AT_FDCWD, "", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
[pid 3287620] openat(AT_FDCWD, "", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
...

To fix, check if the string is empty before calling ReadFile.

This also fixes cases where filePath is non-empty but has extra commas.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 3df9a3c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin requested a review from a team as a code owner April 22, 2022 02:59
@kolyshkin kolyshkin changed the base branch from main to v1 April 22, 2022 02:59
Copy link
Member

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

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

🎉 Thank you!

@meatballhat meatballhat added the area/v1 relates to / is being considered for v1 label Apr 22, 2022
@meatballhat meatballhat added this to the Release 1.22.7 milestone Apr 22, 2022
@meatballhat meatballhat added the kind/bug describes or fixes a bug label Apr 22, 2022
@meatballhat meatballhat changed the title [v1] bad open Don't call open("") backport Apr 22, 2022
Copy link
Member

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

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

and again as @urfave/cli hopefully 🤞🏼

@meatballhat meatballhat merged commit 1b4a05e into urfave:v1 Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v1 relates to / is being considered for v1 kind/bug describes or fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants