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 aws-sdk directly insted of through s3client (fixes #35) #39

Merged
merged 1 commit into from Apr 14, 2020

Conversation

@fmarier
Copy link
Member

fmarier commented Apr 13, 2020

The s3client module is unmaintained and indirectly depends on a vulnerable version of the minimist module. Since it already depends on the aws-sdk module, this change means we can remove a number of dependencies without loss of functionality.

I tested uploads manually using my own AWS account, but I will also trigger a manual rebuild in Jenkins once this is merged and confirm that it works.

The s3client module is unmaintained and indirectly depends on a
vulnerable version of the minimist module. Since it already
depends on the aws-sdk module, this change means we can remove a
number of dependencies without loss of functionality.
@fmarier fmarier self-assigned this Apr 13, 2020
@fmarier fmarier requested review from diracdeltas and jumde Apr 13, 2020
// See: http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Config.html#constructor-property
s3Options: {}
const client = new aws.S3({
maxRetries: 3,

This comment has been minimized.

Copy link
@fmarier

fmarier Apr 13, 2020

Author Member

I wasn't able to find all of the old options since the API has changed significantly. It's not clear to me that they are needed though.

@fmarier fmarier requested a review from bsclifton Apr 13, 2020
Copy link
Member

bsclifton left a comment

Changes LGTM - you tested this with your personal AWS account, right @fmarier? Anything noteworthy about that?

@fmarier
Copy link
Member Author

fmarier commented Apr 14, 2020

Changes LGTM - you tested this with your personal AWS account, right @fmarier? Anything noteworthy about that?

No, it was pretty straightforward. I just had to create the bucket that the upload script is looking for ahead of time and make it public.

@fmarier fmarier merged commit 8987876 into brave:master Apr 14, 2020
1 check passed
1 check passed
Travis CI - Pull Request Build Passed
Details
@fmarier fmarier deleted the fmarier:remove-s3client-35 branch Apr 14, 2020
@fmarier
Copy link
Member Author

fmarier commented Apr 14, 2020

I manually updated from 1.0.15 to 1.0.16 and confirmed that https://https-everywhere.badssl.com/ works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.