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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions