Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

Fix bug: z.stashFile doesn't pick up filename in Content-Disposition #124

Merged
merged 1 commit into from
Oct 29, 2018

Conversation

eliangcs
Copy link
Member

@eliangcs eliangcs commented Oct 23, 2018

Fixes https://github.com/zapier/zapier/issues/14350.

Quoted from the issue:

Developer uses z.stashFile() but when another actions consumes the file (or downloads from the returned S3 URL) it comes with the default unnamedfile filename and not the filename the URL passed to z.stashFile() is sending in the disposition.

return maybeResponse.buffer().then(parseFinalResponse);
return maybeResponse.buffer().then(buffer => {
maybeResponse.dataBuffer = buffer;
return parseFinalResponse(maybeResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

As I'm looking at this diff, it seems that before we were passing parseFinalResponse a buffer object, but not the complete response object. With the change, now we will be giving a complete response object. Am I understanding that correctly? If so, does that have a side effect for other scenarios where we were streaming files?

Mainly concerned with the fact that we'll now be reading off the headers of the response, where as before we didn't touch them. Seems like before and after, we are still leaving the buffer with the response body untouched, so thinking this will be ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you understand it correctly. Without this change, we were always ignoring the response headers if the stream mode is on (z.request({ raw: true })). I'm not sure about the side effect, though. Do you have an example?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not specifically. I was thinking it might mess up .pip() or .read() if somebody was doing that off the response object, but, we've only been passing in the body this whole time, so it shouldn't matter.

@eliangcs eliangcs merged commit 6246096 into master Oct 29, 2018
@eliangcs eliangcs deleted the z.stashFile-filename branch October 29, 2018 04:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants