Repository: mcollina/autocannon (js)

Issue:
Autocannon sends two extra bytes at the end of the body, breaking the protocol
I've tested [uWS](https://github.com/uNetworking/uWebSockets.js) HTTP server and passed little body in every http request. It produced strange results.
I asked (https://github.com/uNetworking/uWebSockets.js/issues/228) creator of uWS why it happens and he researched what _"autocannon sends two extra bytes at the end of the body, breaking the protocol.  uWebSockets does not close the connection immediately, as it has a different parsing strategy. "_

Can you fix it, please?

```
$ autocannon -c 100 -p 100 -d 10 -m POST -b '{"a":1}' http://0.0.0.0:8080
Running 10s test @ http://0.0.0.0:8080
100 connections with 100 pipelining factor

┌─────────┬──────┬──────┬───────┬───────┬──────────┬──────────┬────────────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%   │ Avg      │ Stdev    │ Max        │
├─────────┼──────┼──────┼───────┼───────┼──────────┼──────────┼────────────┤
│ Latency │ 0 ms │ 0 ms │ 56 ms │ 82 ms │ 13.01 ms │ 34.33 ms │ 3618.88 ms │
└─────────┴──────┴──────┴───────┴───────┴──────────┴──────────┴────────────┘
┌───────────┬─────┬──────┬────────┬────────┬────────┬─────────┬─────────┐
│ Stat      │ 1%  │ 2.5% │ 50%    │ 97.5%  │ Avg    │ Stdev   │ Min     │
├───────────┼─────┼──────┼────────┼────────┼────────┼─────────┼─────────┤
│ Req/Sec   │ 0   │ 0    │ 1253   │ 3683   │ 1655.6 │ 1489.25 │ 26      │
├───────────┼─────┼──────┼────────┼────────┼────────┼─────────┼─────────┤
│ Bytes/Sec │ 0 B │ 0 B  │ 231 kB │ 678 kB │ 305 kB │ 274 kB  │ 4.78 kB │
└───────────┴─────┴──────┴────────┴────────┴────────┴─────────┴─────────┘

Req/Bytes counts sampled once per second.

17k requests in 10.15s, 3.05 MB read
7k errors (0 timeouts)
```

Pull request description:
Fix bug with \r\n tail in requests with body
Fix #229 
Also, update tests

Result for uWS
```
$ ./autocannon.js -c 100 -p 100 -d 10 -m POST -b '{"a":1}' http://0.0.0.0:8080
Running 10s test @ http://0.0.0.0:8080
100 connections with 100 pipelining factor

┌─────────┬──────┬──────┬───────┬───────┬─────────┬──────────┬───────────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%   │ Avg     │ Stdev    │ Max       │
├─────────┼──────┼──────┼───────┼───────┼─────────┼──────────┼───────────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 17 ms │ 3.29 ms │ 34.33 ms │ 798.73 ms │
└─────────┴──────┴──────┴───────┴───────┴─────────┴──────────┴───────────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬─────────┬────────┬─────────┐
│ Stat      │ 1%      │ 2.5%    │ 50%     │ 97.5%   │ Avg     │ Stdev  │ Min     │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Req/Sec   │ 26207   │ 26207   │ 30511   │ 34623   │ 30441.6 │ 2745.9 │ 26200   │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Bytes/Sec │ 4.82 MB │ 4.82 MB │ 5.62 MB │ 6.37 MB │ 5.6 MB  │ 505 kB │ 4.82 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴─────────┴────────┴─────────┘

Req/Bytes counts sampled once per second.

304k requests in 10.34s, 56 MB read
```

Patch:
diff --git a/lib/httpRequestBuilder.js b/lib/httpRequestBuilder.js
index eee4693..8ccb589 100644
--- a/lib/httpRequestBuilder.js
+++ b/lib/httpRequestBuilder.js
@@ -1,7 +1,6 @@
 'use strict'
 
 const methods = require('./httpMethods')
-const ending = Buffer.from('\r\n')
 
 // this is a build request factory, that curries the build request function
 // and sets the default for it
@@ -52,7 +51,11 @@ function requestBuilder (defaults) {
       throw new Error(`${method} HTTP method is not supported`)
     }
 
-    const baseReq = `${method} ${path} HTTP/1.1\r\nHost: ${host}\r\nConnection: keep-alive\r\n`
+    const baseReq = [
+      `${method} ${path} HTTP/1.1`,
+      `Host: ${host}`,
+      'Connection: keep-alive'
+    ]
 
     let bodyBuf
 
@@ -71,14 +74,14 @@ function requestBuilder (defaults) {
       headers['Content-Length'] = `${bodyBuf.length + (idCount * 27)}`
     }
 
-    let req = Object.keys(headers)
-      .map((key) => `${key}: ${headers[key]}\r\n`)
-      .reduce((acc, str) => acc + str, baseReq)
+    for (const [key, header] of Object.entries(headers)) {
+      baseReq.push(`${key}: ${header}`)
+    }
 
-    req = Buffer.from(req + '\r\n', 'utf8')
+    let req = Buffer.from(baseReq.join('\r\n') + '\r\n\r\n', 'utf8')
 
     if (bodyBuf && bodyBuf.length > 0) {
-      req = Buffer.concat([req, bodyBuf, ending])
+      req = Buffer.concat([req, bodyBuf])
     }
 
     return req


Test patch:
diff --git a/test/httpClient.test.js b/test/httpClient.test.js
index 538236a..6707b07 100644
--- a/test/httpClient.test.js
+++ b/test/httpClient.test.js
@@ -265,13 +265,13 @@ test('client supports changing the body', (t) => {
   const client = new Client(opts)
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
     'request is okay before modifying')
 
   client.setBody('modified')
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`),
     'body changes updated request')
   client.destroy()
 })
@@ -307,7 +307,7 @@ test('client supports changing the headers and body', (t) => {
   const client = new Client(opts)
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
     'request is okay before modifying')
 
   client.setBody('modified')
@@ -316,7 +316,7 @@ test('client supports changing the headers and body', (t) => {
   })
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`),
     'changes updated request')
   client.destroy()
 })
@@ -331,7 +331,7 @@ test('client supports changing the headers and body together', (t) => {
   const client = new Client(opts)
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
     'request is okay before modifying')
 
   client.setHeadersAndBody({
@@ -339,7 +339,7 @@ test('client supports changing the headers and body together', (t) => {
   }, 'modified')
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`),
     'changes updated request')
   client.destroy()
 })
@@ -354,7 +354,7 @@ test('client supports updating the current request object', (t) => {
   const client = new Client(opts)
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
     'request is okay before modifying')
 
   client.setRequest({
@@ -366,7 +366,7 @@ test('client supports updating the current request object', (t) => {
   })
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+    Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`),
     'changes updated request')
   client.destroy()
 })
@@ -390,11 +390,11 @@ test('client customiseRequest function overwrites the headers and body', (t) =>
   const client = new Client(opts)
 
   t.same(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`),
     'changes updated request')
 
   t.notSame(client.getRequestBuffer(),
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
     'changes updated request')
 
   client.destroy()
@@ -473,7 +473,7 @@ test('client should have 2 different requests it iterates over', (t) => {
     number++
     if (number === 1 || number === 3) {
       t.same(client.getRequestBuffer(),
-        Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`),
+        Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`),
         'body changes updated request')
 
       if (number === 3) {
@@ -482,7 +482,7 @@ test('client should have 2 different requests it iterates over', (t) => {
       }
     } else {
       t.same(client.getRequestBuffer(),
-        Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`),
+        Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`),
         'request was okay')
     }
   })
diff --git a/test/httpRequestBuilder.test.js b/test/httpRequestBuilder.test.js
index b1c2685..8db5634 100644
--- a/test/httpRequestBuilder.test.js
+++ b/test/httpRequestBuilder.test.js
@@ -81,7 +81,7 @@ test('request builder should add a Content-Length header when the body buffer ex
 
   const result = build()
   t.same(result,
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 4\r\n\r\nbody\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 4\r\n\r\nbody`),
     'request is okay')
 })
 
@@ -95,7 +95,7 @@ test('request builder should add a Content-Length header when the body buffer ex
 
   const result = build({ body: 'body' })
   t.same(result,
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 4\r\n\r\nbody\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 4\r\n\r\nbody`),
     'request is okay')
 })
 
@@ -111,7 +111,7 @@ test('request builder should add a Content-Length header with correct calculated
 
   const result = build()
   t.same(result,
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 33\r\n\r\n[<id>]\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 33\r\n\r\n[<id>]`),
     'request is okay')
 })
 
@@ -126,7 +126,7 @@ test('request builder should add a Content-Length header with value "[<contentLe
 
   const result = build({ idReplacement: true })
   t.same(result,
-    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 33\r\n\r\n[<id>]\r\n`),
+    Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 33\r\n\r\n[<id>]`),
     'request is okay')
 })
 
diff --git a/test/requestIterator.test.js b/test/requestIterator.test.js
index ba7c13e..9e4c325 100644
--- a/test/requestIterator.test.js
+++ b/test/requestIterator.test.js
@@ -60,8 +60,8 @@ test('request iterator should create requests with overwritten defaults', (t) =>
     }
   ]
 
-  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`)
-  const request2Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`)
+  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`)
+  const request2Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
 
   opts.requests = requests
 
@@ -96,9 +96,9 @@ test('request iterator should allow for overwriting the requests passed in, but
     }
   ]
 
-  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`)
-  const request2Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`)
-  const request3Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhell0 w0rld\r\n`)
+  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`)
+  const request2Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
+  const request3Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhell0 w0rld`)
 
   opts.requests = requests1
 
@@ -132,10 +132,10 @@ test('request iterator should allow for rebuilding the current request', (t) =>
     }
   ]
 
-  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world\r\n`)
-  const request2Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`)
-  const request3Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified\r\n`)
-  const request4Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified\r\n`)
+  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 11\r\n\r\nhello world`)
+  const request2Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
+  const request3Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
+  const request4Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`)
   const request5Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\n\r\n`)
 
   opts.requests = requests1