vlucas / frisby

Frisby is a REST API testing framework built on Jest that makes testing API endpoints easy, fast, and fun.
http://frisbyjs.com
1.52k stars 201 forks source link

Swagger validation failure for when POSTing a .csv file (multipart/form-data) #494

Open darkshard07 opened 6 years ago

darkshard07 commented 6 years ago

My frisby test (v2) can't seem to get beyond swagger validation with multipart/form-data (file upload)

The swagger description for multipart/form-data is as follows:

paths:
  /importcsv:
    x-swagger-router-controller: order-csv
    post:
      description: Provides ability to import order in csv file format.
      operationId: importCsv
      tags:
        - Staging
      consumes:
        - multipart/form-data
      parameters:
        - name: file
          in: formData
          description: CSV file with orders to be imported
          required: true
          type: file
      responses:
        '200':
          description: Success
          schema:
            $ref: '#/definitions/OrderFileImportResponse'
        default:
          description: error
          schema:
            $ref: '#/definitions/ErrorResponse'

The frisby code/setup is as follows:

const contentPath = path.resolve(__dirname, './order.csv');
let form = frisby.formData();
let content = fs.createReadStream(contentPath);

form.append('file', content, {
      knownLength: fs.statSync(contentPath).size
});

       frisby
                .setup({
                    request: {
                        headers: {
                            'Authorization': sessionToken,
                            'Content-Type': 'multipart/form-data; boundary=' + form.getBoundary(),
                        }
                    }
                })

I've tried a few different ways of POSTing:

1.

.post(importCsvApi, {
    body: form
})

2.

.post(importCsvApi, {
    formData: form
})

3.

.post(importCsvApi, {
    formData: {
        file: form
    }
})

4.

.post(importCsvApi, {
    formData: {
        file: content
    }
})

5.

.post(importCsvApi, {
        formData: {
            file: {
                value: content,
                options: {
                    filename: contentPath,
                    contentType: null
                }
            }
        }
   })

And several other combinations and type of requests, but it always throws the same swagger validation error

Error: Parameter (file) is required
    at throwErrorWithCode (/app/src/node_modules/swagger-tools/lib/validators.js:121:13)
    at Object.module.exports.validateRequiredness (/app/src/node_modules/swagger-tools/lib/validators.js:456:5)
    at /app/src/node_modules/swagger-tools/middleware/swagger-validator.js:334:32
    at /app/src/node_modules/swagger-tools/node_modules/async/lib/async.js:356:13
    at async.forEachOf.async.eachOf (/app/src/node_modules/swagger-tools/node_modules/async/lib/async.js:233:13)
    at _asyncMap (/app/src/node_modules/swagger-tools/node_modules/async/lib/async.js:355:9)
    at Object.map (/app/src/node_modules/swagger-tools/node_modules/async/lib/async.js:337:20)
    at swaggerValidator (/app/src/node_modules/swagger-tools/middleware/swagger-validator.js:321:15)
    at Layer.handle [as handle_request] (/app/src/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/app/src/node_modules/express/lib/router/index.js:312:13)
    at /app/src/node_modules/express/lib/router/index.js:280:7
    at Function.process_params (/app/src/node_modules/express/lib/router/index.js:330:12)
    at next (/app/src/node_modules/express/lib/router/index.js:271:10)
    at setModelsMiddleware (/app/src/app/middlewares/set-models-middleware.js:21:24)
    at <anonymous>

Is this a genuine bug related to frisby communication with swagger or am I doing something wrong? I've tried the same test with the Chakram framework, and it works fine.

            var response = chakram.post(importCsvApi, undefined, {
                headers: {
                    Authorization: sessionToken,
                    'content-type': 'multipart/form-data'
                },
                formData: {
                    file: {
                        value: fs.createReadStream(contentPath),
                        options: {
                            filename: contentPath,
                            contentType: null
                        }
                    }
                }
            });
            expect(response).to.have.status(200);
            return chakram.wait();
H1Gdev commented 6 years ago

@darkshard07

Chakram framework returns Promise(?), but is not it necessary for frisby ?

const contentPath = path.resolve(__dirname, './order.csv');
let form = frisby.formData();
let content = fs.createReadStream(contentPath);

form.append('file', content, {
      knownLength: fs.statSync(contentPath).size
});

return frisby
    .setup({
        request: {
            headers: {
                'Authorization': sessionToken,
                'Content-Type': form.getHeaders()['content-type'],
            }
        }
    })
    .post(importCsvApi, {
        body: form
    });
darkshard07 commented 5 years ago

@H1Gdev The problem here isn't about promises but rather Swagger not accepting the multipart/form-data request that frisby sends. (I only mentioned chakram because swagger accepts requests from chakram).

Like I said, I've tried 'setting up' the request in several way, including the one you specified above, but I still receive the same error

Error: Parameter (file) is required

Some details:

  1. The file to be sent is a simple csv file
  2. The file when sent from swagger docs or from another service with a 'request' object passes swagger validation and is accepted. No such 'Parameter (file) is required' error

Swagger expects a parameter with the name 'file', and my requests subsequently need to have 'file' My swagger def

      parameters:
        - name: file
          in: formData
          description: CSV file with orders to be imported
          required: true
          type: file

I've tried this request, with 'file' as the parameter.

const contentPath = path.resolve(__dirname, './order.csv');
let form = frisby.formData();
let content = fs.createReadStream(contentPath);

form.append('file', content, {
      knownLength: fs.statSync(contentPath).size
});

return frisby
    .setup({
        request: {
            headers: {
                'Authorization': sessionToken,
                'Content-Type': form.getHeaders()['content-type'],
            }
        }
    })
    .post(importCsvApi, {
        file: form
    });

Also, with

    .post(importCsvApi, {
        formData: {
               file: fs.createReadStream(filePath);
         }
    });
H1Gdev commented 5 years ago

@darkshard07

Currently, frisby has this issue on multipart/form-data request. The workaround for this issue is below.

.setup({
  request: {
    headers: {
      'Content-Type': form.getHeaders()['content-type'],
    }
  }
})

So, the following code should correctly send multipart/form-data request like any other framework.

const contentPath = path.resolve(__dirname, './order.csv');
let form = frisby.formData();
let content = fs.createReadStream(contentPath);

form.append('file', content); // content name.

return frisby
    .setup({
        request: {
            headers: {
                'Authorization': sessionToken,
                'Content-Type': form.getHeaders()['content-type'], // workaround.
            }
        }
    })
    .post(importCsvApi, {
        body: form // set to body.
    });

If above workaround does not solve, please cherry-pick PR #495.

imnmi commented 5 years ago

@H1Gdev I think the problem here that @darkshard07 is seeing is different from the one I posted about. The second argument for the post method is automatically merged into req.body, as opposed to being sent in as req.file, or req.formData. Seems like frisby treats all post objects as values for req.body, and there doesn't appear to be a way to customize the input parameter to send formData or file as a separate req parameter, which is why they see the error Error: Parameter (file) is required

H1Gdev commented 5 years ago

@imnmi

Thank you for your opinion. I think that possibility of solving with this PR is low. But can send typical multipart/form-data request.

H1Gdev commented 5 years ago

@darkshard07

Compared to HTTP request on this page, difference is Content-Type of each part.

HTTP request is Content-Type: text/plain. Test code is Content-Type: text/csv.

So, please change Content-Type of each part and do test.

formData.append('file', content, {
    contentType: 'text/plain'
});
imnmi commented 5 years ago

@H1Gdev looks like we found the answer on the page you linked. From the swagger documentation:

The operation payload is defined using formData parameters (not body parameters). The file parameter must have type: file:

If I'm reading this correctly, POST options are always assumed to be body parameter values and passes it along as such, which is why frisby can't pass in a file parameter value and why swagger rejects the request. https://github.com/vlucas/frisby/blob/master/src/frisby/spec.js#L209-L212

// Auto-set 'body' from 'params' JSON if 'body' and 'headers' are not provided (assume sending raw body only)
if (params && _.isUndefined(params.body) && _.isUndefined(params.headers)) {
  postParams.body = JSON.stringify(params);
}

In order to complete the request, frisby needs to be able to handle specifying of non-body type POST params like file.

H1Gdev commented 5 years ago

@darkshard07

FYI

Send this HTTP request using Frisby. (merged PR #495)

code

const frisby = require('frisby');
const path = require('path');
const fs = require('fs');

const contentPath = path.resolve(__dirname, './example.txt');
let form = frisby.formData();
let content = fs.createReadStream(contentPath);

form.append('upfile', content, {
  contentType: 'text/plain',
  knownLength: fs.statSync(contentPath).size
});

frisby.post('http://example.com/upload', {
  body: form
});

example.txt

File contents go here.

HTTP Request

POST /upload HTTP/1.1
User-Agent: frisby/2.1.0 (+https://github.com/vlucas/frisby)
Content-Type: multipart/form-data;boundary=--------------------------304509619808697693429047
Accept: */*
Content-Length: 234
Accept-Encoding: gzip,deflate
Connection: close
Host: example.com

----------------------------304509619808697693429047
Content-Disposition: form-data; name="upfile"; filename="example.txt"
Content-Type: text/plain

File contents go here.

----------------------------304509619808697693429047--
imnmi commented 5 years ago

@H1Gdev In your example the form data is still being passed as a body parameter, which the swagger documentation explicitly calls out as incorrect:

screen shot 2018-10-26 at 9 00 30 am

H1Gdev commented 5 years ago

@imnmi

body parameters and formData(= form) parameters described on this page are defined in the following page.

https://swagger.io/docs/specification/2-0/describing-parameters/ Parameter Types

This is types of parameters in swagger, so it has nothing to do with JavaScript object directly.

imnmi commented 5 years ago

@H1Gdev From your page:

Also, form parameters cannot coexist with the in: body parameter, because formData is a specific way of describing the body.

What frisby appears to be trying to send is:

{
    req: 
        {
            body:
                {
                     formData: "some form data"
                }
        }
}

What you would need based on the specs I'm reading is:

{
    req: 
        {
            formData: "some formdata"
        }
}
H1Gdev commented 5 years ago

@darkshard07 @imnmi

This issue will be fixed by PR #495.

swagger validator may validate Content-Type strictly.

I created a repository for testing. https://github.com/H1Gdev/frisby-issues-494

  1. install. npm install and npm install test-host
  2. start server in localhost using swagger. npm run swagger -- project start test-host
  3. start test using frisby. npm test
darkshard07 commented 5 years ago

@H1Gdev

In your example, you post the formdata in 'body'

.post(importCsvApi, {
            body: form
        })

But according to the swagger documentation, image https://swagger.io/docs/specification/2-0/describing-parameters/#form-parameters

Therefore, shouldn't we post the formdata/file content in 'file' or 'formData' instead of 'body'?

Because with a normal 'request' object, we do use 'formData' and 'file'. And they work with swagger! Examples below:

image https://tanaikech.github.io/2017/07/27/multipart-post-request-using-node.js/

image https://github.com/request/request

I also tried out you testing repo, and am getting a very similar error. Or maybe I'm doing something wrong image

H1Gdev commented 5 years ago

@darkshard07

I also tried out you testing repo, and am getting a very similar error. Or maybe I'm doing something wrong

PR #495 has not yet been merged:sob: So you need to merge PR #495 yourself when test.

Therefore, shouldn't we post the formdata/file content in 'file' or 'formData' instead of 'body'?

Swagger is servers, Frisby(Chakram, Request, etc.) is clients and communicate over HTTP or HTTPS. So it has nothing to do with JavaScript object directly. All we need for File Upload is HTTP multipart/form-data protocol.

a What steps did above occurred ? Please tell me details(or PR on test repo).

I committed outputting request file log. Please fetch and rebase if you use test repo.

vlucas commented 5 years ago

It is merged now :)

H1Gdev commented 5 years ago

@darkshard07

Can you test this ?

elahaha commented 5 years ago

Hello everybody, when trying to run @H1Gdev's example above:

it('should upload file, () => {
  const contentPath = path.resolve(__dirname, '../test_data/file.csv');
  let form = frisby.formData();
  let content = fs.createReadStream(contentPath);

  form.append('file', content, {
    contentType: 'text/csv',
    knownLength: fs.statSync(contentPath).size
  });

  return frisby
      .setup({
          request: {
              headers: {
                  'Content-Type': form.getHeaders()['content-type'] // workaround.
              }
          }
      })
      .post(URL , {
           body: form // set to body.
      })
      .inspectRequest()
      .expect('status', 201)
})

I receive this error output:

MulterError: Unexpected field
    at wrappedFileFilter (C:\node_modules\multer\index.js:40:19)
    at Busboy.<anonymous> (C:\node_modules\multer\lib\make-middleware.js:114:7)
    at Busboy.emit (events.js:189:13)
    at Busboy.emit (C:\node_modules\busboy\lib\main.js:38:33)
    at PartStream.<anonymous> (C:\node_modules\busboy\lib\types\multipart.js:213:13)
    at PartStream.emit (events.js:189:13)
    at HeaderParser.<anonymous> (C:\node_modules\dicer\lib\Dicer.js:51:16)
    at HeaderParser.emit (events.js:189:13)
    at HeaderParser._finish (C:\node_modules\dicer\lib\HeaderParser.js:68:8)
    at SBMH.<anonymous> (C:\node_modules\dicer\lib\HeaderParser.js:40:12)
    at SBMH.emit (events.js:189:13)
    at SBMH._sbmh_feed (C:\node_modules\streamsearch\lib\sbmh.js:95:16)
    at SBMH.push (C:\node_modules\streamsearch\lib\sbmh.js:56:14)
    at HeaderParser.push (C:\node_modules\dicer\lib\HeaderParser.js:46:19)
    at Dicer._oninfo (C:\node_modules\dicer\lib\Dicer.js:197:25)
    at SBMH.<anonymous> (C:\node_modules\dicer\lib\Dicer.js:127:10)
    at SBMH.emit (events.js:189:13)
    at SBMH._sbmh_feed (C:\node_modules\streamsearch\lib\sbmh.js:120:12)
    at SBMH.push (C:\node_modules\streamsearch\lib\sbmh.js:56:14)
    at Dicer._write (C:\node_modules\dicer\lib\Dicer.js:109:17)
    at doWrite (_stream_writable.js:410:12)
  
 at writeOrBuffer (_stream_writable.js:394:5)
    at Dicer.Writable.write (_stream_writable.js:294:11)
    at Multipart.write (C:\node_modules\busboy\lib\types\multipart.js:290:24)
    at Busboy._write (C:\node_modules\busboy\lib\main.js:81:16)
    at doWrite (_stream_writable.js:410:12)
    at writeOrBuffer (_stream_writable.js:394:5)
    at Busboy.Writable.write (_stream_writable.js:294:11)

I can avoid this error by, sending the form as @darkshard07 suggested: .post(URL , form)

but then my request still does not conform to multipart/form-data. .inspectRequest() gives:

  Request: Request {
    size: 0,
    timeout: 5000,
    follow: 20,
    compress: true,
    counter: 0,
    agent: undefined,
    [Symbol(Body internals)]:
     { body:
        <Buffer 7b 22 5f 6f 76 65 72 68 65 61 64 4c 65 6e 67 74 68 22 3a 31 34 38 2c 22 5f 76 61 6c 75 65 4c 65 6e 67 74 68 22 3a 31 31 39 2c 22 5f 76 61 6c 75 65 73 ... >,
       disturbed: false,
       error: null },
    [Symbol(Request internals)]:
     { method: 'POST',
       redirect: 'follow',
       headers: Headers { [Symbol(map)]: [Object] },
       parsedURL:
        Url {
          protocol: 'http:',
          slashes: true,
          auth: null,
          host: 'localhost:8080',
          port: '8080',
          hostname: 'localhost',
          hash: null,
          search: null,
          query: null,
          pathname: '/rest,
          path: '/rest,
          href:
           'http://localhost:8080/rest},
       signal: null } }

and expected req.file remains undefined.

So this is a lot, anyways would be thankful for further hints to solve my issue.

H1Gdev commented 5 years ago

@elahaha

Is Content-Type value ('text/csv') expected by server ?

https://github.com/vlucas/frisby/issues/494#issuecomment-433002641

elahaha commented 5 years ago

@H1Gdev thank you for this very fast reply, actually multipart/form-data is expected as content-type, but exchanging contentType: 'text/csv' with contentType: 'multipart/form-data'

gives me almost the same ouput:

      Request: Request {
        size: 0,
        timeout: 5000,
        follow: 20,
        compress: true,
        counter: 0,
        agent: undefined,
        [Symbol(Body internals)]:
         { body:
            FormData {
              _overheadLength: 159,
              _valueLength: 119,
              _valuesToMeasure: [],
              writable: false,
              readable: true,
              dataSize: 0,
              maxDataSize: 2097152,
              pauseStreams: true,
              _released: true,
              _streams: [],
              _currentStream: null,
              _insideLoop: false,
              _pendingNext: false,
              _boundary: '--------------------------057831990190381541545144',
              _events: [Object],
              _eventsCount: 1 },
           disturbed: false,
           error: null },
        [Symbol(Request internals)]:
         { method: 'POST',
           redirect: 'follow',
           headers: Headers { [Symbol(map)]: [Object] },
           parsedURL:
            Url {
              protocol: 'http:',
              slashes: true,
              auth: null,
              host: 'localhost:8080',
              port: '8080',
              hostname: 'localhost',
              hash: null,
              search: null,
              query: null,
              pathname: '/rest',
              path: '/rest',
              href:
               'http://localhost:8080/rest' },
           signal: null } }
    console.log node_modules/frisby/src/frisby/spec.js:319

      FAILURE Status: 500 
      Body: <!DOCTYPE html>
      <html lang="en">
      <head>
      <meta charset="utf-8">
      <title>Error</title>
      </head>
      <body>
      <pre>MulterError: Unexpected field ....
elahaha commented 5 years ago

Ok I solved it, sorry for the long comments. My problem was that the server expected a key different from 'file'.

Thank you for your answers & have a nice day.