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

Incompatible as a FileSystem with Apache MINA SSHD #56

Closed
rossdrew opened this issue May 24, 2016 · 12 comments
Closed

Incompatible as a FileSystem with Apache MINA SSHD #56

rossdrew opened this issue May 24, 2016 · 12 comments

Comments

@rossdrew
Copy link

rossdrew commented May 24, 2016

Creating an Apache MINA SFTP server and using the S3FileSystemProvider doesn't work as MINA uses this readAttributes

public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException

which is not implemented and results in directories not being listed as expected.

@jarnaiz
Copy link
Member

jarnaiz commented May 24, 2016

Hello Rossdrew,

thanks for your report.

I will try to implement the method this week, i think is not a hard task.

bye!

@rossdrew
Copy link
Author

rossdrew commented May 24, 2016

I've implemented it already in a rather hacky fashion and it then works (at least to list directory contents) with MINA. I'm not really sure the best way to get file permissions though.

@jarnaiz
Copy link
Member

jarnaiz commented May 24, 2016

If you can give me some tips/info about what kind of parameters (LinkOptions and attributes) need to read the Apache MINA. Can help me a lot :)

thanks!

@rossdrew
Copy link
Author

rossdrew commented May 24, 2016

My hacky method (for S3FileAttributes)is as follows. I've hard-coded the permissions as I'm not sure where I would get that information from at this point...

public Map<String, Object> getAsMap(){
    Map<java.lang.String, java.lang.Object> map = new HashMap<>();
    map.put("fileKey",fileKey());
    map.put("isOther",isOther());
    map.put("isRegularFile",isRegularFile());
    map.put("isDirectory",isDirectory());
    map.put("isSymbolicLink",isSymbolicLink());
    map.put("size",size());
    map.put("lastModifiedTime",lastModifiedTime());
    map.put("lastAccessTime",lastAccessTime());
    map.put("permissions", PosixFilePermissions.fromString("rw-rw----"));
    //extended, uig, gid, owner, group, acl didn't seem to be required.

    return map;
}

Without the permissions implemented, MINA will call getFile() which is also unimplemented here and throws another UnsupportedOperationException

@rossdrew rossdrew changed the title Passing this as a FileSystem to Apache MINA causes UnsupportedOperationException Passing as a FileSystem to Apache MINA causes UnsupportedOperationException May 24, 2016
@jarnaiz
Copy link
Member

jarnaiz commented May 28, 2016

Hello Rossdrew,

I hope version 1.3.0 solve your problem :)

Thanks for the feedback!

@jarnaiz jarnaiz closed this as completed May 28, 2016
@rossdrew
Copy link
Author

rossdrew commented May 30, 2016

It doesn't I'm afraid, as, currently

public <A extends BasicFileAttributes> A readAttributes(Path path, Class<A> type, LinkOption... options) throws IOException

doesn't return permissions as an attribute, as required by MINA. As that information is never really collected anywhere. At this point, I'm just running with a hack to readAttributes or S3FileSystemProvider

    @Override
    public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException {
        if (attributes == null) {
            throw new IllegalArgumentException("Attributes null");
        }

        if (attributes.contains(":") && !attributes.contains("basic:")) {
            throw new UnsupportedOperationException(format("attributes %s are not supported, only basic are supported", attributes));
        }

        Map<String, Object> attributeMap = null;
        if (attributes.equals("*") || attributes.equals("basic:*")) {
            BasicFileAttributes attr = readAttributes(path, BasicFileAttributes.class, options);
            attributeMap = AttributesUtils.fileAttributeToMap(attr);
        } else if (attributes.contains(",")) {
            String[] filters = attributes.split(",");
            BasicFileAttributes attr = readAttributes(path, BasicFileAttributes.class, options);
            attributeMap = AttributesUtils.fileAttributeToMap(attr, filters);
        }

        if (attributeMap != null) {
            //XXX Hack as I have no idea how to get Amazon permissions
            attributeMap.put("permissions", PosixFilePermissions.fromString("rw-rw----"));
            return attributeMap;
        }

        throw new UnsupportedOperationException();
    }

FYI: There are also other reasons this wont work with MINA, as I've found out. For example, it' requires an implementation of S3FileChannel in order to do an FTP GET.

@jarnaiz
Copy link
Member

jarnaiz commented May 30, 2016

Ups...
I think I missundertand you :/

What its the subclass needed for Apache MINA? s3fs only support BasicFileAttributes and S3FileAttributes and dont have the key "permissions".

In your first comment you talk to me about the method:

public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException

And now:

public <A extends BasicFileAttributes> A readAttributes(Path path, Class<A> type, LinkOption... options) throws IOException

Can you writte me some test or spike so we can reproduce your expected behaviour?

Sorry!

@jarnaiz jarnaiz reopened this May 30, 2016
@rossdrew
Copy link
Author

rossdrew commented May 30, 2016

Yes, it requires the Map<String, Object> return type but your new implementation calls the one with the <A extends BasicFileAttributes> return type, which has no permissions

I was unaware that permissions isn't included in basic attributes however. It seems I have bigger problems here as Apache MINA SSHD requires the permissions attribute.

My project is pretty small if you fancy running it to improve your project
I just provide my own S3FilesystemFactory to MINAs SftpSubsystem via SshServer in my SFTPService class and MINA SSHD does the rest. The intention was to create an SFTP front-end for S3.

@rossdrew rossdrew changed the title Passing as a FileSystem to Apache MINA causes UnsupportedOperationException Incompatible as a FileSystem with Apache MINA SSHD Jun 3, 2016
@rossdrew
Copy link
Author

rossdrew commented Jun 3, 2016

So FYI, I've gotten it working with MINA SSHD with a few hacks. Namely adding permissions and a FileChannel option. I would submit a pull request but it's a little too hacky, I have a feeling you'll be able to do this better than me :)

public class S3FileSystemProviderWithFileChannel extends S3FileSystemProvider {
    /**
     * Just a FileChannel interface that points to a SeekableByteChannel
     */
    @Override
    public FileChannel newFileChannel(Path path,
                                      Set<? extends OpenOption> options,
                                      FileAttribute<?>... attrs)
            throws IOException
    {
        S3Path s3Path = toS3Path(path);
        return new S3FileChannel(s3Path, options);
    }

    /**
     * Private in original library so needs to be duplicated
     */
    private S3Path toS3Path(Path path) {
        Preconditions.checkArgument(path instanceof S3Path, "path must be an instance of %s", S3Path.class.getName());
        return (S3Path) path;
    }

    @Override
    public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException {
        Map<String, Object> attributeMap = super.readAttributes(path, attributes, options);

        if (attributeMap != null) {
            //XXX Hack as I have no idea how to get Amazon permissions as S3ObjectSummary (Amazon AWS SDK) doesn't contain any
            attributeMap.put("permissions", PosixFilePermissions.fromString("rw-rw----"));
        }

        return attributeMap;
    }
}

jarnaiz added a commit that referenced this issue Aug 10, 2016
Added initial simple implementation and not completed.
@jarnaiz
Copy link
Member

jarnaiz commented Aug 11, 2016

Hi @rossdrew

I implemented the method readAttributes with PosixFileAttributes.class in tag 1.4.0. The conversion between S3ACLPermission and PosixPermission is very simple and limitated but I think you can use it without any workaround in your project and the implementation can be improved in the future.

cheers!

@jarnaiz jarnaiz closed this as completed Aug 11, 2016
@robmoore
Copy link

robmoore commented Jan 30, 2017

I noticed that S3FileSystem still has code which doesn't include posix as a member.

    @Override
    public Set<String> supportedFileAttributeViews() {
        return ImmutableSet.of("basic");
    }

When the Mina sshd code attempts to check file attributes, it only sees basic so it skips the code path for posix attributes and fails due to an unimplemented toFile() method in S3Path. It's calling toFile() as a fallback since it tries to see whether it can get them from File instead.

It seems like posix should be added as an option.

@robmoore
Copy link

Just wanted to bump this issue to make sure my comment was seen. Should I create another ticket?

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

No branches or pull requests

3 participants