Skip to content

Commit

Permalink
fix multipart/related requests when uploading multiple attachments. F…
Browse files Browse the repository at this point in the history
…ixes issue#238 (#279)

Co-authored-by: Glynn Bird <glynnbird@apache.org>
  • Loading branch information
glynnbird and Glynn Bird authored Sep 13, 2021
1 parent 8552352 commit 4290e55
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 7 deletions.
53 changes: 53 additions & 0 deletions lib/multipart.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const CRLF = '\r\n'
const DASHES = '--'

// generate the payload, boundary and header for a multipart/related request
// to upload binary attachments to CouchDB.
// https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html
class MultiPartFactory {
// constructor
constructor (parts) {
// generate a unique id that forms the boundary between parts
this.boundary = this.uuid()
const bufferList = []

// for each part to be processed
for (const part of parts) {
// start with the boundary e.g. --0559337432997171\r\n
bufferList.push(Buffer.from(DASHES + this.boundary + CRLF))

// state the type and length of the following part
bufferList.push(Buffer.from(`content-type: ${part.content_type}${CRLF}`))
bufferList.push(Buffer.from(`content-length: ${part.data.length}${CRLF}`))

// two \r\n marks start of the part itself
bufferList.push(Buffer.from(CRLF))

// output the string/buffer
bufferList.push(typeof part.data === 'string' ? Buffer.from(part.data) : part.data)

// followed by /r/n
bufferList.push(Buffer.from(CRLF))
}

// right at the end we have an end marker e.g. --0559337432997171--\r\n
bufferList.push(Buffer.from(DASHES + this.boundary + DASHES + CRLF))

// buid up a single Buffer from the array of bits
this.data = Buffer.concat(bufferList)

// calculate the Content-Type header required to send with this request
this.header = `multipart/related; boundary=${this.boundary}`
}

uuid () {
const chars = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'.split('')
let retval = ''
for (let i = 0; i < 16; i++) {
retval += chars[Math.floor(Math.random() * chars.length)]
}
return retval
}
}

module.exports = MultiPartFactory
14 changes: 10 additions & 4 deletions lib/nano.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const SCRUBBED_STR = 'XXXXXX'
const defaultHttpAgent = new http.Agent(AGENT_DEFAULTS)
const defaultHttpsAgent = new https.Agent(AGENT_DEFAULTS)
const ChangesReader = require('./changesreader.js')
const MultiPartFactory = require('./multipart.js')

function isEmpty (val) {
return val == null || !(Object.keys(val) || val).length
Expand Down Expand Up @@ -282,7 +283,11 @@ module.exports = exports = function dbScope (cfg) {
}

if (opts.multipart) {
req.multipart = opts.multipart
// generate the multipart/related body, header and boundary to
// upload multiple binary attachments in one request
const mp = new MultiPartFactory(opts.multipart)
opts.contentType = mp.header
req.body = mp.data
}

req.headers = Object.assign(req.headers, opts.headers, cfg.defaultHeaders)
Expand Down Expand Up @@ -891,12 +896,13 @@ module.exports = exports = function dbScope (cfg) {
/* jscs:disable requireCamelCaseOrUpperCaseIdentifiers */
content_type: att.content_type
}
multipart.push({ body: att.data })
multipart.push(att)
})

multipart.unshift({
'content-type': 'application/json',
body: JSON.stringify(doc)
content_type: 'application/json',
data: JSON.stringify(doc),
name: 'document'
})

return relax({
Expand Down
9 changes: 6 additions & 3 deletions test/multipart.insert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ afterEach(() => {
test('should be able to insert a document with attachments #1 - multipart PUT /db/id - db.multipart.insert', async () => {
// mocks
const response = { ok: true, id: '8s8g8h8h9', rev: '1-123' }
const scope = nock(COUCH_URL, { reqheaders: { 'content-type': h => h.includes('multipart/related') } })
const scope = nock(COUCH_URL)
.matchHeader('content-type', h => h.includes('multipart/related'))
.put('/db/docid')
.reply(200, response)

Expand All @@ -66,7 +67,8 @@ test('should be able to insert a document with attachments #1 - multipart PUT /d

test('should be able to insert a document with attachments #2 - multipart PUT /db/id - db.multipart.insert', async () => {
const response = { ok: true, id: '8s8g8h8h9', rev: '1-123' }
const scope = nock(COUCH_URL, { reqheaders: { 'content-type': h => h.includes('multipart/related') } })
const scope = nock(COUCH_URL)
.matchHeader('content-type', h => h.includes('multipart/related'))
.put('/db/docid')
.reply(200, response)

Expand All @@ -83,7 +85,8 @@ test('should be able to handle 404 - db.multipart.insert', async () => {
error: 'not_found',
reason: 'missing'
}
const scope = nock(COUCH_URL, { reqheaders: { 'content-type': h => h.includes('multipart/related') } })
const scope = nock(COUCH_URL)
.matchHeader('content-type', h => h.includes('multipart/related'))
.put('/db/docid')
.reply(404, response)

Expand Down
56 changes: 56 additions & 0 deletions test/multipart.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
const MultiPartFactory = require('../lib/multipart.js')
const textAttachment = { name: 'test.txt', data: 'Hello\r\nWorld!', content_type: 'text/plain' }
const anotherTextAttachment = { name: 'test2.txt', data: 'the quick brown fox', content_type: 'text/plain' }

test('should return different boundary each time', async () => {
const mf1 = new MultiPartFactory([])
const mf2 = new MultiPartFactory([])
const mf3 = new MultiPartFactory([])

expect(typeof mf1.boundary).toBe('string')
expect(typeof mf2.boundary).toBe('string')
expect(typeof mf3.boundary).toBe('string')
expect(mf1.boundary.length).toBe(16)
expect(mf2.boundary.length).toBe(16)
expect(mf3.boundary.length).toBe(16)
expect(mf1).not.toEqual(mf2)
expect(mf1).not.toEqual(mf3)
expect(mf2).not.toEqual(mf3)
})

test('should return boundary in header', async () => {
const mf1 = new MultiPartFactory([])
const boundary = mf1.boundary
const header = mf1.header
expect(header).toEqual(`multipart/related; boundary=${boundary}`)
})

test('should handle single attachments', async () => {
const mf1 = new MultiPartFactory([textAttachment])
expect(typeof mf1.data).toEqual('object')
expect(Buffer.isBuffer(mf1.data)).toEqual(true)
const lines = mf1.data.toString().split('\r\n')
expect(lines).toContain(`--${mf1.boundary}`)
expect(lines).toContain('content-type: text/plain')
expect(lines).toContain('content-length: 13')
expect(lines).toContain('')
expect(lines).toContain('Hello')
expect(lines).toContain('World!')
expect(lines).toContain(`--${mf1.boundary}--`)
})

test('should handle two attachments', async () => {
const mf1 = new MultiPartFactory([textAttachment, anotherTextAttachment])
expect(typeof mf1.data).toEqual('object')
expect(Buffer.isBuffer(mf1.data)).toEqual(true)
const lines = mf1.data.toString().split('\r\n')
expect(lines).toContain(`--${mf1.boundary}`)
expect(lines).toContain('content-type: text/plain')
expect(lines).toContain('content-length: 13')
expect(lines).toContain('')
expect(lines).toContain('Hello')
expect(lines).toContain('World!')
expect(lines).toContain('content-length: 19')
expect(lines).toContain('the quick brown fox')
expect(lines).toContain(`--${mf1.boundary}--`)
})

0 comments on commit 4290e55

Please sign in to comment.