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

Use visibility argument when ::copy() in AWS adapters #1547

Merged
merged 1 commit into from Sep 17, 2022

Conversation

fnash
Copy link

@fnash fnash commented Sep 7, 2022

Avoids get-object-acl operation and s3:GetObjectAcl permission.

Copy link

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting to suppress a call to S3 api when the visibility can be specified.

  1. Is it possible to add a test?
  2. Is there other adapters to update to ensure a consistent behavior?

src/AsyncAwsS3/AsyncAwsS3Adapter.php Outdated Show resolved Hide resolved
src/AwsS3V3/AwsS3V3Adapter.php Outdated Show resolved Hide resolved
@fnash fnash force-pushed the avoid-get-object-acl-in-copy branch from 2c5eea4 to cb2a108 Compare September 7, 2022 16:12
@fnash fnash requested a review from GromNaN September 7, 2022 16:14
@fnash
Copy link
Author

fnash commented Sep 7, 2022

@frankdejonge Is it Okay for you?

@fnash fnash force-pushed the avoid-get-object-acl-in-copy branch from cb2a108 to fcfa49d Compare September 9, 2022 19:34
@frankdejonge
Copy link
Member

@fnash can you base this PR on 3.x please?

@fnash
Copy link
Author

fnash commented Sep 10, 2022

Okay!

But need support for php7.4. Do I make another PR for 2.x?

@frankdejonge
Copy link
Member

@fnash 2.x is in security fixes only mode as of a couple of months.

@frankdejonge
Copy link
Member

I can see if I can make an exception, but that exposes me to more people who want an exception ... :/

@GromNaN
Copy link

GromNaN commented Sep 15, 2022

That's kind of security issue since the visibility that is specified is not respected.

@frankdejonge
Copy link
Member

I think you're exaggerating the issue now, but I'll look it I have time for it.

@frankdejonge frankdejonge merged commit bc7bac3 into thephpleague:2.x Sep 17, 2022
@fnash fnash deleted the avoid-get-object-acl-in-copy branch September 20, 2022 07:01
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

Successfully merging this pull request may close these issues.

None yet

3 participants