Merge pull request #841 from apiaryio/honzajavorek/multipart-fix

Modify multipart body and calculate Content-Length in the right order

src/transaction-runner.coffee

@@ -553,6 +553,9 @@ class TransactionRunner
   # An actual HTTP request, before validation hooks triggering
   # and the response validation is invoked here
   performRequestAndValidate: (test, transaction, hooks, callback) ->
+    if transaction.request.body and @isMultipart(transaction.request.headers)
+      transaction.request.body = @fixApiBlueprintMultipartBody(transaction.request.body)
+
     @setContentLength(transaction)
     requestOptions = @getRequestOptionsFromTransaction(transaction)
 
@@ -586,10 +589,6 @@ class TransactionRunner
 
           @validateTransaction test, transaction, callback
 
-
-    if transaction.request['body'] and @isMultipart requestOptions
-      @replaceLineFeedInBody transaction, requestOptions
-
     try
       @performRequest(requestOptions, handleRequest)
     catch error
@@ -674,17 +673,19 @@ class TransactionRunner
         transaction.test = test
         return callback()
 
-  isMultipart: (requestOptions) ->
-    caseInsensitiveRequestHeaders = {}
-    for key, value of requestOptions.headers
-      caseInsensitiveRequestHeaders[key.toLowerCase()] = value
-    caseInsensitiveRequestHeaders['content-type']?.indexOf("multipart") > -1
-
-  replaceLineFeedInBody: (transaction, requestOptions) ->
-    if transaction.request['body'].indexOf('\r\n') == -1
-      transaction.request['body'] = transaction.request['body'].replace(/\n/g, '\r\n')
-      transaction.request['headers']['Content-Length'] = Buffer.byteLength(transaction.request['body'], 'utf8')
-      requestOptions.headers = transaction.request['headers']
+  isMultipart: (headers) ->
+    contentType = caseless(headers).get('Content-Type')
+    if contentType
+      contentType.indexOf('multipart') > -1
+    else
+      false
+
+  # Finds newlines not preceeded by carriage returns and replaces them by
+  # newlines preceeded by carriage returns.
+  #
+  # See https://github.com/apiaryio/api-blueprint/issues/401
+  fixApiBlueprintMultipartBody: (body) ->
+    body.replace(/\r?\n/g, '\r\n')
 
 
 module.exports = TransactionRunner

test/fixtures/request/application-json.apib

@@ -0,0 +1,17 @@
+FORMAT: 1A
+
+# Testing 'application/json' Request API
+
+# POST /data
+
++ Request (application/json)
+
+    + Body
+
+            {"test": 42}
+
++ Response 200 (application/json; charset=utf-8)
+
+    + Body
+
+            {"test": "OK"}

test/fixtures/request/application-x-www-form-urlencoded.apib

@@ -0,0 +1,17 @@
+FORMAT: 1A
+
+# Testing 'application/x-www-form-urlencoded' Request API
+
+# POST /data
+
++ Request (application/x-www-form-urlencoded)
+
+    + Body
+
+            test=42
+
++ Response 200 (application/json; charset=utf-8)
+
+    + Body
+
+            {"test": "OK"}

test/fixtures/request/multipart-form-data.apib

@@ -0,0 +1,27 @@
+FORMAT: 1A
+
+# Testing 'multipart/form-data' Request API
+
+# POST /data
+
++ Request (multipart/form-data;boundary=---BOUNDARY)
+
+    + Body
+
+            ---BOUNDARY
+            Content-Disposition: form-data; name="text"
+            Content-Type: text/plain
+
+            test equals to 42
+            ---BOUNDARY
+            Content-Disposition: form-data; name="json"; filename="filename.json"
+            Content-Type: application/json
+
+            {"test": 42}
+            ---BOUNDARY--
+
++ Response 200 (application/json; charset=utf-8)
+
+    + Body
+
+            {"test": "OK"}

test/fixtures/request/text-plain.apib

@@ -0,0 +1,17 @@
+FORMAT: 1A
+
+# Testing 'text/plain' Request API
+
+# POST /data
+
++ Request (text/plain)
+
+    + Body
+
+            test equals to 42
+
++ Response 200 (application/json; charset=utf-8)
+
+    + Body
+
+            {"test": "OK"}

test/integration/helpers.coffee

@@ -90,6 +90,7 @@ getSSLCredentials = ->
 #     - *endpointUrl*: 0 (number, default) - number of requests to the endpoint
 createServer = (options = {}) ->
   protocol = options.protocol or 'http'
+  bodyParserInstance = options.bodyParser or bodyParser.json({size: '5mb'})
 
   serverRuntimeInfo =
     requestedOnce: false
@@ -99,7 +100,7 @@ createServer = (options = {}) ->
     requestCounts: {}
 
   app = express()
-  app.use(bodyParser.json({size: '5mb'}))
+  app.use(bodyParserInstance)
   app.use((req, res, next) ->
     recordServerRequest(serverRuntimeInfo, req)
     res.type('json').status(200) # sensible defaults, can be overriden

test/integration/request-test.coffee

@@ -0,0 +1,159 @@
+{assert} = require('chai')
+bodyParser = require('body-parser')
+
+{runDreddWithServer, createServer} = require('./helpers')
+Dredd = require('../../src/dredd')
+
+
+describe('Sending \'application/json\' request', ->
+  runtimeInfo = undefined
+  contentType = 'application/json'
+
+  beforeEach((done) ->
+    app = createServer({bodyParser: bodyParser.text({type: contentType})})
+    app.post('/data', (req, res) ->
+      res.json({test: 'OK'})
+    )
+
+    path = './test/fixtures/request/application-json.apib'
+    dredd = new Dredd({options: {path}})
+
+    runDreddWithServer(dredd, app, (err, info) ->
+      runtimeInfo = info
+      done(err)
+    )
+  )
+
+  it('results in one request being delivered to the server', ->
+    assert.isTrue(runtimeInfo.server.requestedOnce)
+  )
+  it('the request has the expected Content-Type', ->
+    assert.equal(runtimeInfo.server.lastRequest.headers['content-type'], contentType)
+  )
+  it('the request has the expected format', ->
+    body = runtimeInfo.server.lastRequest.body
+    assert.deepEqual(JSON.parse(body), {test: 42})
+  )
+  it('results in one passing test', ->
+    assert.equal(runtimeInfo.dredd.stats.tests, 1)
+    assert.equal(runtimeInfo.dredd.stats.passes, 1)
+  )
+)
+
+
+describe('Sending \'multipart/form-data\' request', ->
+  runtimeInfo = undefined
+  contentType = 'multipart/form-data'
+
+  beforeEach((done) ->
+    path = './test/fixtures/request/multipart-form-data.apib'
+
+    app = createServer({bodyParser: bodyParser.text({type: contentType})})
+    app.post('/data', (req, res) ->
+      res.json({test: 'OK'})
+    )
+    dredd = new Dredd({options: {path}})
+
+    runDreddWithServer(dredd, app, (err, info) ->
+      runtimeInfo = info
+      done(err)
+    )
+  )
+
+  it('results in one request being delivered to the server', ->
+    assert.isTrue(runtimeInfo.server.requestedOnce)
+  )
+  it('the request has the expected Content-Type', ->
+    assert.include(runtimeInfo.server.lastRequest.headers['content-type'], 'multipart/form-data')
+  )
+  it('the request has the expected format', ->
+    assert.equal(runtimeInfo.server.lastRequest.body, [
+      '---BOUNDARY'
+      'Content-Disposition: form-data; name="text"'
+      'Content-Type: text/plain'
+      ''
+      'test equals to 42'
+      '---BOUNDARY'
+      'Content-Disposition: form-data; name="json"; filename="filename.json"'
+      'Content-Type: application/json'
+      ''
+      '{"test": 42}'
+      '---BOUNDARY--'
+      ''
+    ].join('\r\n'))
+  )
+  it('results in one passing test', ->
+    assert.equal(runtimeInfo.dredd.stats.tests, 1)
+    assert.equal(runtimeInfo.dredd.stats.passes, 1)
+  )
+)
+
+
+describe('Sending \'application/x-www-form-urlencoded\' request', ->
+  runtimeInfo = undefined
+  contentType = 'application/x-www-form-urlencoded'
+
+  beforeEach((done) ->
+    path = './test/fixtures/request/application-x-www-form-urlencoded.apib'
+
+    app = createServer({bodyParser: bodyParser.text({type: contentType})})
+    app.post('/data', (req, res) ->
+      res.json({test: 'OK'})
+    )
+    dredd = new Dredd({options: {path}})
+
+    runDreddWithServer(dredd, app, (err, info) ->
+      runtimeInfo = info
+      done(err)
+    )
+  )
+
+  it('results in one request being delivered to the server', ->
+    assert.isTrue(runtimeInfo.server.requestedOnce)
+  )
+  it('the request has the expected Content-Type', ->
+    assert.equal(runtimeInfo.server.lastRequest.headers['content-type'], contentType)
+  )
+  it('the request has the expected format', ->
+    assert.equal(runtimeInfo.server.lastRequest.body, 'test=42\n')
+  )
+  it('results in one passing test', ->
+    assert.equal(runtimeInfo.dredd.stats.tests, 1)
+    assert.equal(runtimeInfo.dredd.stats.passes, 1)
+  )
+)
+
+
+describe('Sending \'text/plain\' request', ->
+  runtimeInfo = undefined
+  contentType = 'text/plain'
+
+  beforeEach((done) ->
+    path = './test/fixtures/request/text-plain.apib'
+
+    app = createServer({bodyParser: bodyParser.text({type: contentType})})
+    app.post('/data', (req, res) ->
+      res.json({test: 'OK'})
+    )
+    dredd = new Dredd({options: {path}})
+
+    runDreddWithServer(dredd, app, (err, info) ->
+      runtimeInfo = info
+      done(err)
+    )
+  )
+
+  it('results in one request being delivered to the server', ->
+    assert.isTrue(runtimeInfo.server.requestedOnce)
+  )
+  it('the request has the expected Content-Type', ->
+    assert.equal(runtimeInfo.server.lastRequest.headers['content-type'], contentType)
+  )
+  it('the request has the expected format', ->
+    assert.equal(runtimeInfo.server.lastRequest.body, 'test equals to 42\n')
+  )
+  it('results in one passing test', ->
+    assert.equal(runtimeInfo.dredd.stats.tests, 1)
+    assert.equal(runtimeInfo.dredd.stats.passes, 1)
+  )
+)