Skip to content

[core-http] fix An exception thrown on Safari 5.0#11759

Merged
jeremymeng merged 1 commit into
Azure:masterfrom
ewfian:master
Nov 4, 2020
Merged

[core-http] fix An exception thrown on Safari 5.0#11759
jeremymeng merged 1 commit into
Azure:masterfrom
ewfian:master

Conversation

@ewfian
Copy link
Copy Markdown
Contributor

@ewfian ewfian commented Oct 10, 2020

Fixes #11758

When uploading files on Safari 5.0, An exception body must be a string, Blob, ArrayBuffer, ArrayBufferView, or a function returning NodeJS.ReadableStream. will thrown.

@ghost ghost added Azure.Core customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Oct 10, 2020
@ghost
Copy link
Copy Markdown

ghost commented Oct 10, 2020

Thank you for your contribution ewfian! We will review the pull request and get back to you soon.

@ewfian ewfian mentioned this pull request Oct 10, 2020
6 tasks
Copy link
Copy Markdown
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

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

The fix looks good to me, though can you confirm if the different way of doing this in core-client also works in Safari 5?

!(value?.constructor?.name === "Blob")

@ewfian
Copy link
Copy Markdown
Contributor Author

ewfian commented Oct 13, 2020

The fix looks good to me, though can you confirm if the different way of doing this in core-client also works in Safari 5?

!(value?.constructor?.name === "Blob")

@xirzec There is no above code line in js file what I bundled. But the result of next is as follows. it seems won't be work.

!(typeof Blob === "function" && value instanceof Blob)

image
image

Copy link
Copy Markdown
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@bterlson
Copy link
Copy Markdown
Member

Concerned that we will not have a good error message now if a user passes in a vanilla object, since we'll now try to do an instanceof check using it.

@xirzec
Copy link
Copy Markdown
Member

xirzec commented Oct 13, 2020

Concerned that we will not have a good error message now if a user passes in a vanilla object, since we'll now try to do an instanceof check using it.

This shouldn't affect the value being passed in, the only thing that seems to be changing is how we detect that the global name Blob is present. Arguably, we could have inverted it with typeof(Blob) !== "undefined"

@jeremymeng
Copy link
Copy Markdown
Member

@bterlson any more concerns?

@bterlson
Copy link
Copy Markdown
Member

Sorry, no concerns, @xirzec is right that I misread the situation.

@jeremymeng jeremymeng changed the title fix An exception thrown on Safari 5.0 [core-http] fix An exception thrown on Safari 5.0 Nov 4, 2020
@jeremymeng jeremymeng merged commit 7218452 into Azure:master Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core customer-reported Issues that are reported by GitHub users external to the Azure organization.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An exception thrown on Safari 5.0

4 participants