Skip to content

Bug: wrong error message returned by Client.Remove #468

@awaken

Description

@awaken

Hi,
Great job! I really appreciate your effort for maintaining this library!
I mainly use your library inside a proprietary Big Data project, that is running in production since 2 years ago, using Linux VMs in the cloud, and moves many TBs of data per day.

func (c *Client) Remove(path string) error can return an error for many reasons, of course.
In most cases, the library just works fine.

If Remove is called for removing just a file (and not a directory), in some cases I experienced that Remove can return an error containing this message: "Not a directory".
In my opinion, it is a bug.

Looking at the Remove implementation, I see that, in case err.Code is equal to sshFxFailure or if there is a permission error, the returned error is the one got from RemoveDirectory.
I think if Remove is called for removing a file, it should never return an error with a message like "Not a directory".

I would suggest this fixed implementation:

func (c *Client) Remove(path string) error {
	err := c.removeFile(path)
	// some servers, *cough* osx *cough*, return EPERM, not ENODIR.
	// serv-u returns ssh_FX_FILE_IS_A_DIRECTORY
	// EPERM is converted to os.ErrPermission so it is not a StatusError
	if err2, ok := err.(*StatusError); ok {
		switch err2.Code {
		case sshFxFailure:
			if c.RemoveDirectory(path) == nil { return nil }
			return err
		case sshFxFileIsADirectory:
			return c.RemoveDirectory(path)
		}
	}
	if os.IsPermission(err) {
		if c.RemoveDirectory(path) == nil { return nil }
	}
	return err
}

Thanks for your attention.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions