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

Relax constraints on objects that we'll accept as a file #766

Merged
merged 1 commit into from
Apr 16, 2019

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Apr 16, 2019

When uploading a file, we previously made a check on whether it
supported both #read and #path before wrapping it in a
Faraday::UploadIO and sending it off. The second check on #path
isn't strictly necessary, and as reported in #761 can prevent objects
created by open-uri from being compatible with file upload.

Here we remove the check #path so that we just require that objects
support #read, and in addition error when an object is passed that's
not file-compatible and not a string. This should prevent users from
seeing the very confusing "Invalid hash" error in these situations.

Fixes #761.

r? @ob-stripe
cc @stripe/api-libraries

When uploading a file, we previously made a check on whether it
supported both `#read` and `#path` before wrapping it in a
`Faraday::UploadIO` and sending it off. The second check on `#path`
isn't strictly necessary, and as reported in #761 can prevent objects
created by `open-uri` from being compatible with file upload.

Here we remove the check `#path` so that we just require that objects
support `#read`, and in addition error when an object is passed that's
not file-compatible and not a string. This should prevent users from
seeing the very confusing "Invalid hash" error in these situations.

Fixes #761.
@brandur-stripe brandur-stripe force-pushed the brandur-relax-file-constraints branch from b761820 to 81b7cb3 Compare April 16, 2019 15:33
Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Brandur.

@brandur-stripe
Copy link
Contributor

Thanks OB! Taking this out.

@brandur-stripe brandur-stripe merged commit c7db5de into master Apr 16, 2019
@brandur-stripe brandur-stripe deleted the brandur-relax-file-constraints branch April 16, 2019 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stripe::InvalidRequestError when trying to create Stripe File
4 participants