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.

Sign up for GitHub

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

Jump to bottom

Bug: Notation panics on init when $HOME and $XDG_CONFIG_HOME unset #446

Closed
JasonTheDeveloper opened this issue Aug 30, 2024 · 0 comments · Fixed by #449
Closed

Bug: Notation panics on init when $HOME and $XDG_CONFIG_HOME unset #446

JasonTheDeveloper opened this issue Aug 30, 2024 · 0 comments · Fixed by #449
Assignees
@JasonTheDeveloper
Labels
bug Something isn't working

Comments

@JasonTheDeveloper
Copy link
Contributor

What is not working as expected?

The bug relates to an issue raised in fluxcd/flux2#4961

This gist of the issue is when $HOME or $XDC_CONFIG_HOME are unset, dir/path panics when initialising

notation-go/dir/path.go

Lines 82 to 97 in 974c291

func init() {
loadUserPath()
}
// loadUserPath function defines UserConfigDir and UserLibexecDir.
func loadUserPath() {
// set user config
userDir, err := userConfigDir()
if err != nil {
panic(err)
}
UserConfigDir = filepath.Join(userDir, notation)
// set user libexec
UserLibexecDir = UserConfigDir
}

What did you expect to happen?

I would expect notation to not panic, breaking any application importing the notation SDK when $HOME and $XDC_CONFIG_HOME are unset.

How can we reproduce it?

Sets can be found in the reference issue

Describe your environment

OS: Arch Linux, Debian Bookworm
Golang: 1.22.5

What is the version of your notation-go Library?

v1.1.0
@JasonTheDeveloper JasonTheDeveloper added bug Something isn't working triage Needs evaluation for feasibility, timeline, etc. labels Aug 30, 2024
@JasonTheDeveloper JasonTheDeveloper mentioned this issue Sep 2, 2024
Merged
@Two-Hearts Two-Hearts linked a pull request Sep 3, 2024 that will close this issue
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set #449
Merged
@yizha1 yizha1 removed the triage Needs evaluation for feasibility, timeline, etc. label Sep 3, 2024
JeyJeyGao pushed a commit that referenced this issue Sep 4, 2024
@JasonTheDeveloper @shizhMSFT @Two-Hearts
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (#…
4d76f9a 
…449)

This PR addresses the issue #446

In this PR I:

- I removed the `init()` function from `dir/path`
- When `userConfigDir()` returns an error, instead of `panic(err)` I
default to the current directory instead
- Split `loadUserPath()` into two new functions used to setup and return
the values for `UserConfigDir` and `UserLibexecDir`
- Added additional unit tests for the two new functions and to test the
default directory is used when `HOME` is set to `""`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: JasonTheDeveloper <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Co-authored-by: Patrick Zheng <[email protected]>
JeyJeyGao pushed a commit to JeyJeyGao/notation-go that referenced this issue Sep 4, 2024
@JasonTheDeveloper @shizhMSFT
@Two-Hearts @JeyJeyGao
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (n…
eb08659 
…otaryproject#449)

This PR addresses the issue notaryproject#446

In this PR I:

- I removed the `init()` function from `dir/path`
- When `userConfigDir()` returns an error, instead of `panic(err)` I
default to the current directory instead
- Split `loadUserPath()` into two new functions used to setup and return
the values for `UserConfigDir` and `UserLibexecDir`
- Added additional unit tests for the two new functions and to test the
default directory is used when `HOME` is set to `""`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: JasonTheDeveloper <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Co-authored-by: Patrick Zheng <[email protected]>
@JeyJeyGao JeyJeyGao mentioned this issue Sep 4, 2024
Merged
Two-Hearts added a commit to Two-Hearts/notation-go that referenced this issue Sep 4, 2024
@JasonTheDeveloper @shizhMSFT @Two-Hearts
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (n…
0e4fbf0 
…otaryproject#449)

This PR addresses the issue notaryproject#446

In this PR I:

- I removed the `init()` function from `dir/path`
- When `userConfigDir()` returns an error, instead of `panic(err)` I
default to the current directory instead
- Split `loadUserPath()` into two new functions used to setup and return
the values for `UserConfigDir` and `UserLibexecDir`
- Added additional unit tests for the two new functions and to test the
default directory is used when `HOME` is set to `""`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: JasonTheDeveloper <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Co-authored-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts mentioned this issue Sep 4, 2024
Merged
JeyJeyGao pushed a commit to JeyJeyGao/notation-go that referenced this issue Sep 4, 2024
@JasonTheDeveloper @shizhMSFT
@Two-Hearts @JeyJeyGao
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (n…
73b4bf6 
…otaryproject#449)

This PR addresses the issue notaryproject#446

In this PR I:

- I removed the `init()` function from `dir/path`
- When `userConfigDir()` returns an error, instead of `panic(err)` I
default to the current directory instead
- Split `loadUserPath()` into two new functions used to setup and return
the values for `UserConfigDir` and `UserLibexecDir`
- Added additional unit tests for the two new functions and to test the
default directory is used when `HOME` is set to `""`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: JasonTheDeveloper <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Co-authored-by: Patrick Zheng <[email protected]>
JeyJeyGao added a commit that referenced this issue Sep 5, 2024
@JeyJeyGao @JasonTheDeveloper
@shizhMSFT @Two-Hearts
fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (#…
e6c7fef 
…449)  (#450)

This PR addresses the issue
#446 for
`release-1.1` branch
This commit was cherry picked from commit
4d76f9a


In this PR I:

- I removed the `init()` function from `dir/path`
- When `userConfigDir()` returns an error, instead of `panic(err)` I
default to the current directory instead
- Split `loadUserPath()` into two new functions used to setup and return
the values for `UserConfigDir` and `UserLibexecDir`
- Added additional unit tests for the two new functions and to test the
default directory is used when `HOME` is set to `""`

Signed-off-by: Jason <[email protected]>
Signed-off-by: JasonTheDeveloper <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Co-authored-by: JasonTheDeveloper <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Co-authored-by: Patrick Zheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JasonTheDeveloper JasonTheDeveloper

Labels
bug Something isn't working
Projects
Notary Project Planning Board
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set JasonTheDeveloper/notation-go
2 participants
@JasonTheDeveloper @yizha1