mitchellh/go-homedir

Directorys starting with ~something throw an error

Snapstromegon opened this issue · 2 comments

In a MR in containers/common someone mentioned this package after I already reimplemented this module.
While looking into this, I found a bug in your code:
homedir.Expand("~something")

This throws an "cannot expand user-specific home dir". Directory names starting with a "~" are valid, so this is a bug in my opinion.

I implemented the function (using os/user) as follows:

// resolveHomeDir converts a path referencing the home directory via "~"
// to an absolute path
func resolveHomeDir(path string) (string, error) {
	// check if the path references the home dir to avoid work
	// don't use strings.HasPrefix(path, "~") as this doesn't match "~" alone
	// use strings.HasPrefix(...) to not match "something/~/something"
	if !(path == "~" || strings.HasPrefix(path, "~/")) {
		// path does not reference home dir -> Nothing to do
		return path, nil
	}

	// only get HomeDir when necessary
	home, err := unshare.HomeDir()
	if err != nil {
		return "", err
	}

	// replace the first "~" (start of path) with the HomeDir to resolve "~"
	return strings.Replace(path, "~", home, 1), nil
}

By replacing unshare.HomeDir with your Dir function, this issue should be fixed.

This is not a bug, if I'm understanding correctly.

Shells treat ~USER as a shorthand for getting the home directory for a different user. We've had users in projects I've helped create that use this (Terraform, Vault, etc.) that attempt to do this. This error helps explain that that does not work.

As you said, ~foo is a valid directory name in many cases too. I'm not sure how to reconcile these two use cases. In the past I've just recommended people do the kind of check that you have if you want to allow paths like that.

Another option may be to have a function that returns the type of directory it is and let the end user handle the logic. I think the core error here is not a bug at this point though.

A see your trade-off, and therefore close this issue.