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

Enable writings node.js buffers directly #680

Closed
wants to merge 2 commits into from

Conversation

EricMCornelius
Copy link

Hi there,

I ran into encoding problems with binary request string data (gzipped payloads) being transformed via Buffer.toString('utf8') in transformRequest.

This PR will enable passing in node buffers directly instead of needing to create a stream or copy to an ArrayBuffer just for the sake of sending compressed data.

Copy link
Member

@nickuraltsev nickuraltsev left a comment

Choose a reason for hiding this comment

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

Could you please add a test for this?

@@ -73,6 +73,6 @@
},
"typings": "./index.d.ts",
"dependencies": {
"follow-redirects": "1.0.0"
"follow-redirects": "0.0.7"
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary?

@EricMCornelius
Copy link
Author

Sorry, I believe the downgrade was because I ran into a serious regression w/ the updated follow-redirects dependency and updated my master branch of my fork.

Using the updated follow-redirects causes this reported write after end issue: follow-redirects/follow-redirects#50 - which I was hitting in the component using my axios fork. I haven't had time to investigate that dependency defect further.

@EricMCornelius
Copy link
Author

#623 - I see there's already some awareness of the problem with follow-redirects as well.

@jcready
Copy link
Contributor

jcready commented Mar 23, 2017

I'm also running into the problem with follow-redirects. Setting maxRedirects: 0 fixes the problem, but is obviously not ideal.

@jcready
Copy link
Contributor

jcready commented Mar 23, 2017

It looks like the problem with follow-redirects was fixed in 1.1.0 but axios locks down the version to 1.0.0.

@rubennorte
Copy link
Member

Sorry but we've finally merged #773 as it seemed more complete. Thanks for your contribution.

@rubennorte rubennorte closed this Apr 8, 2017
@jcready
Copy link
Contributor

jcready commented Apr 8, 2017

@rubennorte what about the issue with follow-redirects? Should that be opened as a separate PR?

@rubennorte
Copy link
Member

@jcready yes please

@axios axios locked and limited conversation to collaborators May 3, 2020
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.

None yet

4 participants