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

TransferUitlity should have ACL option for uploading file (needs 2 request to upload a public-read file) #63

Closed
fernando-closs-clansoft opened this issue Oct 6, 2015 · 41 comments
Labels
feature-request Request a new feature

Comments

@fernando-closs-clansoft
Copy link

The transfer utility is fantastic, but there is too much limitation on changing the putobjectrequest, it is only possible to set the metadata. Something like the TransferUtilityUploadRequest (as in .Net sdk) should be also available in Android.

To upload a public-read file using it, i need to handle the successful upload, then set the ACL, then finally notify the success on uploading the file. One unnecessary extra request, but i wouldn't like to lose all the benefits of the TransferService and Transfer Utility.

Thanks for your attention.

@ghost
Copy link

ghost commented Oct 6, 2015

+1
Java also has support for this as shown in http://docs.aws.amazon.com/AmazonS3/latest/dev/acl-using-java-sdk.html
Using the Android SDK would be the best, however.

@fernando-closs-clansoft
Copy link
Author

The PutObjectRequest has the ACL Option (withCannedAcl) but the TransferUtility and its UploadTask does not have an option to set it, so it will be null. =Z

@wdane
Copy link
Contributor

wdane commented Oct 6, 2015

Thanks for the request guys, I'll keep this issue open and I've added this to our feature request list. If others feel like this is a feature they want please +1 so we can adjust our priorities accordingly!

That being said AWS in general recommends using IAM policies or bucket policies see http://blogs.aws.amazon.com/security/post/TxPOJBY6FE360K/IAM-policies-and-Bucket-Policies-and-ACLs-Oh-My-Controlling-Access-to-S3-Resourc that being said, I understand there are situations where having an ACL might be easier.

@angelromero
Copy link

+1
It's an important feature.

2015-10-06 14:29 GMT-05:00 wdane [email protected]:

Thanks for the request guys, I'll keep this issue open and I've added this
to our feature request list. If others feel like this is a feature they
want please +1 so we can adjust our priorities accordingly!


Reply to this email directly or view it on GitHub
#63 (comment).

Angel Romero Salazar

@pedro-almeida-afterverse

+1
I also need this feature!

@claudio-jeremias-clansoft

+1
Nice feature!!!

@fernando-closs-clansoft
Copy link
Author

Thanks for the so fast reply, wdane!

@fpg1503
Copy link

fpg1503 commented Oct 6, 2015

+1
This would help a lot!

@gannonlawlor
Copy link

+1

@fosterzhang fosterzhang added the feature-request Request a new feature label Oct 20, 2015
@jitendraramoliya
Copy link

+1
This Feature really require. make this feature ASAP so User can use this feature

@niteshgoel
Copy link

+1
Any alternate, so that i can make all the url as public? As I need it very urgently.

@raul1991
Copy link

raul1991 commented Nov 7, 2015

So till this feature request is implemented, Is there way around to this situation ?

@fosterzhang
Copy link
Contributor

A work around is that you can apply bucket policy to the bucket you upload objects. See Bucket Policy Examples. The following bucket policy grants read only permission to objects under bar and foobar path in the bucket foo.

{
  "Version":"2012-10-17",
  "Statement":[
    {
      "Sid":"AddPerm",
      "Effect":"Allow",
      "Principal": "*",
      "Action":["s3:GetObject"],
      "Resource":["arn:aws:s3:::foo/bar/*","arn:aws:s3:::foo/foobar/*"]
    }
  ]
}

See my original answer on StackOverflow How to specify public-read policy on a complete bucket in aws?.

@fernando-closs-clansoft
Copy link
Author

The problem with this solution is that you change the whole bucket policy when you just want to change one object ACL. And if you want to change only one file you have to do an extra call to set ACL while it could be already set on the object in the upload.

My solution was not to use TransferUtility, which is sad because it has a good implementation on a separate service, etc, but i've used the PutObjectRequest inside an AsyncTask. Here is the code:

private class UploadFileTask extends AsyncTask<Void, Long, Exception> {

        private FileUploadCallback callback;
        private AWSCredentials credentialsProvider;
        private String fileUrl;
        private boolean publicRead;
        private String bucket;
        private FileUploader uploaderInstance; //used to store client for reuse in same Uploader Instance
        private AmazonS3Client s3Client = null;
        private boolean cancelled = false;
        private long totalSize = 0;
        private long totalTransferred = 0;

        public UploadFileTask(AmazonS3Client s3Client, String fileUrl, String bucket,boolean publicRead, FileUploadCallback callback, AWSCredentials credentialsProvider, FileUploader instanceReference)
        {
            this.s3Client = s3Client;
            this.fileUrl = fileUrl;
            this.bucket = bucket;
            this.publicRead = publicRead;
            this.credentialsProvider = credentialsProvider;
            this.callback = callback;
            this.uploaderInstance = instanceReference;
        }
        protected Exception doInBackground(Void... urls) {
            //Instantiate and store reference on uploader Instance
            if(s3Client == null) {
                s3Client = new AmazonS3Client(credentialProvider);
                s3Client.setS3ClientOptions(new S3ClientOptions().withPathStyleAccess(true));

                s3Client.setRegion(defaultRegion);

                uploaderInstance.setS3Client(s3Client);
            }



            File f = new File(fileUrl);
            try {
                totalSize = f.length();
            }
            catch (Exception e)
            {
                return e;
            }

            PutObjectRequest por = new PutObjectRequest(this.bucket,f.getName(),f);
            if(this.publicRead)
            {
                por.setCannedAcl(CannedAccessControlList.PublicRead);
            }

            por.setGeneralProgressListener(new ProgressListener() {
                @Override
                public void progressChanged(ProgressEvent progressEvent) {
                    //Log.i("PROGRESS",progressEvent.getEventCode() + " - " + progressEvent.getBytesTransferred());
                    if(progressEvent.getEventCode() == 0)
                    {
                        totalTransferred += progressEvent.getBytesTransferred();
                        publishProgress(totalTransferred);
                    }
                    else if(progressEvent.getEventCode() == ProgressEvent.CANCELED_EVENT_CODE)
                    {
                        cancelled = true;
                    }

                }
            });

            try {
                s3Client.putObject(por);

                //Notify success
                return null;
            }
            catch (Exception e)
            {
                //Notify error
                return e;
            }


        }

        protected void onProgressUpdate(Long... progress) {
            if(callback!=null)
            {
                callback.onUploadProgressChanged(progress[0].longValue(),totalSize);
            }
        }

        protected void onPostExecute(Exception exception) {
            if(callback!=null) {
                if (exception == null) {
                    if(!cancelled) {
                        callback.onUploadSuccess();
                    }
                    else
                    {
                        callback.onUploadCanceled();
                    }
                }
                else
                {
                    callback.onUploadError(exception);
                }
            }
        }
    }

@fosterzhang
Copy link
Contributor

@fernando-closs-clansoft that should do the job :).

What I proposed is an alternate solution and is applicable most of the time. There are a few benefits.

  • Better management of objects as they are grouped by certain prefixes (or directories). For example, public objects go to public/ and private ones go to private/.
  • Easy to change access permission. Simply update the bucket policy.

Just my two cents.

Back to the original topic, I'll leave this issue open as feature request.

@fernando-closs-clansoft
Copy link
Author

@fosterzhang I completely agree with you. In my case there was no way to have this directory structure due to retro compatibility with an existing iOs app. But this is a good solution until we have the ACL option in the feature. =)

@avdheshyadav
Copy link

+1

@erichkleung
Copy link

+1, would rather not have to use 2 calls if using TransferUtility.

@nemanjavuk
Copy link

+1

3 similar comments
@yurii-diachenko
Copy link

+1

@jeandavid
Copy link

+1

@vault101
Copy link

+1

@Mrzy
Copy link

Mrzy commented Jan 29, 2016

+1!!!!!

@shanuka
Copy link

shanuka commented Feb 2, 2016

@SarthakM9
Copy link

+1

2 similar comments
@Suresh1988
Copy link

+1

@shroge
Copy link

shroge commented Feb 10, 2016

+1

@jhermida-agero
Copy link

+1

6 similar comments
@musticmen
Copy link

+1

@hsyadav733
Copy link

+1

@eligreenfeld
Copy link

+1

@schrockblock
Copy link

+1

@sibelius
Copy link

sibelius commented Mar 4, 2016

👍

@foteros
Copy link

foteros commented Mar 7, 2016

+1

@anuragopenxcell
Copy link

+1 , been months and still waiting.

@Irfan55121
Copy link

+1

@esodot
Copy link

esodot commented Apr 12, 2016

please please add it

@fosterzhang
Copy link
Contributor

Hi all, this feature has been added to v2.2.15. See release notes.

@sibelius
Copy link

thanks @fosterzhang, please close this

@gaojiusong
Copy link

+1, would rather not have to use 2 calls if using TransferUtility.

@nnbanerjee
Copy link

Is it implemented? Why a feature? It is a bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request a new feature
Projects
None yet
Development

No branches or pull requests