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

[BUG 100% reproducible] Crash when creating non-existing file without permissions for it #3080

Closed
smac89 opened this issue Dec 7, 2023 · 6 comments · Fixed by #3082
Closed

Comments

@smac89
Copy link
Contributor

smac89 commented Dec 7, 2023

Description of the problem or steps to reproduce

As the title says, attempting to create a new file which requires sudo permissions to save, results in the above error.

  1. Open micro (without sudo)
  2. Write something
  3. Save the file in a path which requires sudo permissions
  4. Observe the following crash
runtime/panic.go:261 (0x55e610494898)
runtime/panic.go:260 (0x55e610494866)
github.com/zyedidia/micro/v2/internal/action/actions.go:805 (0x55e6108e56f3)
github.com/zyedidia/micro/v2/internal/info/infobuffer.go:152 (0x55e6108bd1a5)
github.com/zyedidia/micro/v2/internal/action/infopane.go:206 (0x55e6108fd39f)
github.com/zyedidia/micro/v2/internal/action/infopane.go:54 (0x55e6108fc571)
github.com/zyedidia/micro/v2/internal/action/infopane.go:129 (0x55e6108fcd0f)
github.com/zyedidia/micro/v2/internal/action/infopane.go:93 (0x55e6108fc9ee)
github.com/zyedidia/micro/v2/cmd/micro/micro.go:474 (0x55e6109371d4)
github.com/zyedidia/micro/v2/cmd/micro/micro.go:394 (0x55e610936a8c)
runtime/internal/atomic/types.go:194 (0x55e610480172)
runtime/asm_amd64.s:1650 (0x55e6104b16e1)

Note that I have the "mkparents": true option enabled.

Specifications

Commit hash: 68d88b5
OS: ArchLinux
Terminal: Konsole

@dustdfg
Copy link
Contributor

dustdfg commented Dec 7, 2023

The problem is in two functions: SaveAsCB and saveBufToFile

As said in stacktrace error is caused on line 805:

fmt.Sprintf("the file %s already exists in the directory, would you like to overwrite? Y/n", fileinfo.Name()),

fileinfo.Name() doesn't "exist" if two errors (fs.ErrPermission and fs.ErrNotExist) occur at the same time.

SaveAsCB function checks if file not exist and if it doesn't exist it calls saveBufToFile (which internally checks for file permission) but it doesn't matter because it will return false to noPrompt variable, function won't reach "if-ed" return and will go to line 805 where will try to use fileinfo.Name() that can't work...

fileinfo, err := os.Stat(filename)
if err != nil {
if os.IsNotExist(err) {
noPrompt := h.saveBufToFile(filename, action, callback)
if noPrompt {
h.completeAction(action)
return
}
}
}

@dustdfg
Copy link
Contributor

dustdfg commented Dec 7, 2023

The logic of checks is separate between two functions. One tries to check if file exist another tries to check if user have permissions for the file. One of them calls another. Looks like we have two checks and everything should work normally but it isn't so. I don't like this code. And Callbacks they are messy here...

I think logic of checks should be in one place not in two and that it should be flattened off callbacks (but it looks like prompting architecture doesn't allow it)

@smac89 smac89 changed the title [BUG 100% reproducible] invalid memory address or nil pointer dereference [BUG 100% reproducible] "invalid memory address or nil pointer dereference" when creating a non-existing file Dec 7, 2023
@dustdfg
Copy link
Contributor

dustdfg commented Dec 8, 2023

You can't save file when it doesn't exist but you have permissions? You described in issue (and I checked it is really so) that error occurs when you try to save non existing file without permissions for it. I would recommend the following issue name: Crash when creating non-existing file without permissions for it

@JoeKar
Copy link
Collaborator

JoeKar commented Dec 8, 2023

}
InfoBar.YNPrompt(
fmt.Sprintf("the file %s already exists in the directory, would you like to overwrite? Y/n", fileinfo.Name()),
func(yes, canceled bool) {
if yes && !canceled {
noPrompt := h.saveBufToFile(filename, action, callback)
if noPrompt {
h.completeAction(action)
}
}
},
)

Why not simply adding an else around the YNPrompt?

Because if I understood the code from saveBufToFile right, then we could trigger the same issue in the moment autosu is disabled, but we don't want to save it. Afterwards the next prompt should be triggered by telling us, that the file already exists?
This wouldn't happen then and fileinfo.Name() should be accessible.

@dustdfg
Copy link
Contributor

dustdfg commented Dec 8, 2023

Why not simply adding an else around the YNPrompt?

Do you mean:

} else {
  InfoBar.YNPrompt(
      //.....
  )
}

Because it doesn't work ¯\_(ツ)_/¯ Edit: it was my local branch with previous attempts to fix it (,,>﹏<,,). So now I have a question why it works with else?

P.S If you want to embed the code not just link, you need to strip L<number>-C<number> to just L<number>

@JoeKar
Copy link
Collaborator

JoeKar commented Dec 8, 2023

P.S If you want to embed the code not just link, you need to strip L-C to just L

Ok, next time. The GitHub help wasn't sufficient for that.

Do you mean:

Yes.

[...] why it works with else?

Because then I'd expect err == nil and fileinfo set, which then allows the message printed with the YN prompt and this is then the only moment it should be printed, because it really exists. In the original scenario it tries to print that with no reason (because it doesn't need to exist yet) and fails.

@smac89 smac89 changed the title [BUG 100% reproducible] "invalid memory address or nil pointer dereference" when creating a non-existing file [BUG 100% reproducible] Crash when creating non-existing file without permissions for it Jan 17, 2024
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 a pull request may close this issue.

3 participants