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.

Sign up for GitHub

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

Jump to bottom

panic on nil deref in S3 downloader (Content-Range missing?) #417

Closed
philpennock opened this issue Nov 1, 2015 · 3 comments
Closed

panic on nil deref in S3 downloader (Content-Range missing?) #417

philpennock opened this issue Nov 1, 2015 · 3 comments
Labels
bug This issue is a bug.

Comments

@philpennock
Copy link

We're seeing Concourse's S3 file management fail as of a few days ago when managing artifacts with S3. Across a set of 7 files, at least two will always fail and panic inside aws-sdk-go.

As far as I can tell, the code is assuming that the Content-Range header will always be returned from AWS. Since the code is requesting chunks of 5MB (by default, not touched by Concourse), for files smaller than 5MB this header isn't necessarily present.

It looks to me like this might be a change in behaviour by S3 causing the SDK to break.

I suspect that the correct fix is to test for presence of the .ContentRange field (*string not nil) and if absent, then check if .ContentLength is non-nil and use that instead, else defer until the content has been read?

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x531305]

goroutine 7 [running]:
github.com/aws/aws-sdk-go/service/s3/s3manager.(*downloader).setTotalBytes(0xc820096380, 0xc8201360e0)
    /tmp/build/b6237934-9e2f-4838-56a4-73bd768b9a80/gopath/src/github.com/concourse/s3-resource/Godeps/_workspace/src/github.com/aws/aws-sdk-go/service/s3/s3manager/download.go:206 +0x95
github.com/aws/aws-sdk-go/service/s3/s3manager.(*downloader).downloadPart(0xc820096380, 0xc8200146c0)
    /tmp/build/b6237934-9e2f-4838-56a4-73bd768b9a80/gopath/src/github.com/concourse/s3-resource/Godeps/_workspace/src/github.com/aws/aws-sdk-go/service/s3/s3manager/download.go:175 +0x46a
created by github.com/aws/aws-sdk-go/service/s3/s3manager.(*downloader).download
    /tmp/build/b6237934-9e2f-4838-56a4-73bd768b9a80/gopath/src/github.com/concourse/s3-resource/Godeps/_workspace/src/github.com/aws/aws-sdk-go/service/s3/s3manager/download.go:114 +0xbe

https://github.com/concourse/s3-resource
@krobertson krobertson mentioned this issue Nov 2, 2015
Closed
@jasdel
Copy link
Contributor

jasdel commented Nov 2, 2015

Hi @philpennock thanks for getting in contact with us. I've been able to reproduce the issue you reported. It looks to occur anytime a Object is downloaded with the S3 Download manager which is less than 5MB. Is this consistent with what you are seeing?

I think you're correct that the best change is if ContentRange is nil grab ContentLength instead.

@jasdel jasdel added investigating This issue is being investigated and/or work is in progress to resolve the issue. bug This issue is a bug. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Nov 2, 2015
@jasdel
Copy link
Contributor

jasdel commented Nov 2, 2015

@philpennock I just pushed 2c52a85 which fixes this issue. Thanks for reporting this issue!

@jasdel jasdel closed this as completed Nov 2, 2015
@philpennock
Copy link
Author

thanks for the quick fix.

jasdel added a commit that referenced this issue Nov 3, 2015
@jasdel
service/s3/s3manager: Add S3 Download Manager non-chunk test
2a76bd5 
Adds test a case for non-chunked download manager.

Fix #417
jasdel added a commit that referenced this issue Nov 3, 2015
@jasdel
Tag release v0.10.2
aed0bed 
References:
  #417
skotambkar added a commit to skotambkar/aws-sdk-go that referenced this issue May 20, 2021
@skotambkar
internal/sdkio: add RingBuffer data structure to the sdk (aws#417)
f6d7c84 
Adds a RingBuffer data structure. RingBuffer acts as a revolving buffer of a predefined length. It implements io.ReadWriter interface.
skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this issue May 20, 2021
@skmcgrail
Release v0.19.0 (2020-01-30) (aws#480)
dac592f 
Breaking Change
---
* `service`: Add generated service for wafregional and dynamodbstreams aws#463
  * Updates the wafregional and dynamodbstreams API clients to include all API operations, and types that were previously shared between waf and dynamodb API clients respectively. This update ensures that all API clients include all operations and types needed for that client, and shares no types with another client package.
  * To migrate your applications to use the updated wafregional and dynamodbstreams you'll need to update the package the impacted type is imported from to match the client the type is being used with.
* `aws`: Context has been added to EC2Metadata operations.([aws#461](aws/aws-sdk-go-v2#461))
  * Also updates utilities that directly or indirectly depend on EC2Metadata client. Signer utilities, credential providers now take in context.
* `private/model`: Add utility for validating shape names for structs and enums for the service packages ([aws#471](aws/aws-sdk-go-v2#471))
  * Fixes bug which allowed service package structs, enums to start with non alphabetic character
  * Fixes the incorrect enum types in mediapackage service package, changing enum types __AdTriggersElement, __PeriodTriggersElement to AdTriggersElement, PeriodTriggersElement respectively.
* `aws`: Client, Metadata, and Request structures have been refactored to simplify the usage of resolved endpoints ([aws#473](aws/aws-sdk-go-v2#473))
  * `aws.Client.Endpoint` struct member has been removed, and `aws.Request.Endpoint` struct member has been added of type `aws.Endpoint`
  * `aws.Client.Region` structure member has been removed

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws`: `PartitionID` has been added to `aws.Endpoint` structure, and is used by the endpoint resolver to indicate which AWS partition an endpoint was resolved for ([aws#473](aws/aws-sdk-go-v2#473))
* `aws/endpoints`: Updated resolvers to populate `PartitionID` for a resolved `aws.Endpoint` ([aws#473](aws/aws-sdk-go-v2#473))
* `service/s3`: Add support for Access Point resources
  * Adds support for using Access Point resource with Amazon S3 API operation calls. The Access Point resource are identified by an Amazon Resource Name (ARN).
  * To make operation calls to an S3 Access Point instead of a S3 Bucket, provide the Access Point ARN string as the value of the Bucket parameter. You can create an Access Point for your bucket with the Amazon S3 Control API. The Access Point ARN can be obtained from the S3 Control API. You should avoid building the ARN directly.

SDK Enhancements
---
* `internal/sdkio`: Adds RingBuffer data structure to the sdk [aws#417](aws/aws-sdk-go-v2#417)
  * Adds an implementation of RingBuffer data structure which acts as a revolving buffer of a predefined length. The RingBuffer implements io.ReadWriter interface.
  * Adds unit tests to test the behavior of the ring buffer.
* `aws/ec2metadata`: Adds support for EC2Metadata client to use secure tokens provided by the IMDS ([aws#453](aws/aws-sdk-go-v2#453))
  * Modifies EC2Metadata client to use request context within its operations ([aws#462](aws/aws-sdk-go-v2#462))
  * Reduces the default dialer timeout and response header timeout to help reduce latency for known issues with EC2Metadata client running inside a container
  * Modifies and adds tests to verify the behavior of the EC2Metadata client.
* `service/dynamodb/dynamodbattribute`: Adds clarifying docs on dynamodbattribute.UnixTime ([aws#464](aws/aws-sdk-go-v2#464))
* `example/service/sts/assumeRole`: added sts assume role example ([aws#224](aws/aws-sdk-go-v2#224))
  * Fixes [aws#157](aws/aws-sdk-go-v2#157) by adding an example for Amazon STS assume role to retrieve credentials.

SDK Bugs
---
* `service/dynamodb/dynamodbattribute`: Fixes a panic when decoding into a map with a key string type alias. ([aws#465](aws/aws-sdk-go-v2#465))
  * Fixes [aws#410](aws/aws-sdk-go-v2#410),  by adding support for keys that are string aliases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue is a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@philpennock @jasdel