-
Notifications
You must be signed in to change notification settings - Fork 20
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
bugfix: don't use req.Marshal to encode a binary payload… #110
Conversation
…ncorrectly assumes source type (which is actually a byteslice) from content-type
… incorrectly assumes the destination type (which is a byteslice) from the content-type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment because resp.Body
can be nil - but otherwise LGTM 👍
storage/2023-11-03/blob/blobs/get.go
Outdated
result.HttpResponse = resp.Response | ||
|
||
err = resp.Unmarshal(result.Contents) | ||
respBody, err := io.ReadAll(resp.Body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's possible for resp.Body
to be nil here fwiw, so this is a potential crash-point:
respBody, err := io.ReadAll(resp.Body) | |
var respBody io.Reader | |
if resp.Body != nil { | |
respBody, err = io.ReadAll(resp.Body) |
storage/2023-11-03/blob/blobs/get.go
Outdated
} | ||
resp.Body.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resp.Body.Close() | |
respBody.Close() | |
} |
… since this incorrectly assumes source type (which is actually a byteslice) from content-type
related: hashicorp/terraform-provider-azurerm#25333