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

formdata gets stringified #447

Closed
simllll opened this issue Oct 10, 2021 · 5 comments · Fixed by #475
Closed

formdata gets stringified #447

simllll opened this issue Oct 10, 2021 · 5 comments · Fixed by #475
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@simllll
Copy link

simllll commented Oct 10, 2021

Environment details

  • OS: linux
  • Node.js version: v16.10
  • npm version: 7.24.1
  • gaxios version: 4.3.2

Steps to reproduce

send a form dat areuqest

const formData = new FormData();
formData.append('test', 123);

gaxios.request({method: POST, data: formData;})

The request is stringiefied because of this line here

} else if (typeof opts.data === 'object') {

formdata is typeof object.

a possible fix will be to check if object is a formdata first and in this case don't touch it?
e.g.

  else if (typeof FormData !== 'undefined' && opts.data instanceof FormData) {
// remove content-type header...  
} else if (typeof opts.data === 'object') { ...

altenative would be to allow skipping the whole transformation step in the validate step.. e.g. via flag?

@simllll
Copy link
Author

simllll commented Oct 11, 2021

This only happens on client side btw

@SurferJeffAtGoogle SurferJeffAtGoogle added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Oct 11, 2021
@simllll
Copy link
Author

simllll commented Oct 11, 2021

I was playing around with this issue, and right now I patched it like this:
https://github.com/hokify/haxios/blob/cd3e244a69d1b949821c6dfdcf4d23d2e8c774b3/src/index.ts#L97-L111

I acutally "inverted" the check, now I only stringify objects to json if they are plain javascripts objects, and let the others as they are:

export function isPlainObject(obj) {
	const prototype = Object.getPrototypeOf(obj);
	return (
		prototype === null ||
		prototype.constructor === Object ||
		prototype.constructor === null ||
		prototype.toString() === '[object Object]'
	);
}

this is even supported by ie11.

@bcoe bcoe added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. web and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Oct 13, 2021
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jan 11, 2022
@tyayers
Copy link

tyayers commented Jan 13, 2022

I think this is also causing problems for me when trying to post form binary data to the Apigee API to deploy proxies (zip files), makes sense that it doesn't work if all form data is stringified..

Here's my code (using google-auth-library-nodejs, which uses gaxios):

const form = new FormData();
var newFile = fs.readFileSync(bundlePath);
form.append('file', newFile, `${proxyName + ".zip"}`);

client.request({
  url: `https://apigee.googleapis.com/v1/organizations/${projectId}/apis?name=${proxyName}&action=import`,
  method: "post",
  headers: {
    ...form.getHeaders()
  },
  data: form
})

And the error:
Error: invalid multipart/form-data encoding: failed to read multipart body part: multipart: NextPart: bufio: buffer full
at Gaxios._request (node_modules/gaxios/src/gaxios.ts:158:15)
at processTicksAndRejections (internal/process/task_queues.js:95:5)
at async JWT.requestAsync (node_modules/google-auth-library/build/src/auth/oauth2client.js:368:18)

Works fine with axios, so switched back to that for now.

@sofisl
Copy link
Contributor

sofisl commented Apr 8, 2022

Hi @simllll, I have a PR open to fix this issue. One thing I'm not sure about is that you mention that data is stringified because of this line, but that actually stringifies the body. Did you mean to say that the body gets stringified, which you want fixed? Just to confirm, I've added an assertion here that confirms we do nothing to the body, which is what I'm assuming you're looking for. LMK if that looks right to you.

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 8, 2022
@sofisl
Copy link
Contributor

sofisl commented Apr 8, 2022

I'm going to merge since this issue has gone OOSLO. Feel free to reopen if we need to make any other changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants