Skip to content

Commit

Permalink
fix: return content length mismatch error
Browse files Browse the repository at this point in the history
  • Loading branch information
Simone Sanfratello committed Apr 24, 2021
1 parent 9f6bf86 commit ffc97e8
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 56 deletions.
28 changes: 16 additions & 12 deletions docs/api/Errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ You can find all the error objects inside the `errors` key.
const { errors } = require('undici')
```

| Error | Error Codes | Description |
| -----------------------------|-----------------------------------|------------------------------------------------|
| `InvalidArgumentError` | `UND_ERR_INVALID_ARG` | passed an invalid argument. |
| `InvalidReturnValueError` | `UND_ERR_INVALID_RETURN_VALUE` | returned an invalid value. |
| `RequestAbortedError` | `UND_ERR_ABORTED` | the request has been aborted by the user |
| `ClientDestroyedError` | `UND_ERR_DESTROYED` | trying to use a destroyed client. |
| `ClientClosedError` | `UND_ERR_CLOSED` | trying to use a closed client. |
| `SocketError` | `UND_ERR_SOCKET` | there is an error with the socket. |
| `NotSupportedError` | `UND_ERR_NOT_SUPPORTED` | encountered unsupported functionality. |
| `ContentLengthMismatchError` | `UND_ERR_CONTENT_LENGTH_MISMATCH`| body does not match content-length header |
| `InformationalError` | `UND_ERR_INFO` | expected error with reason |
| `TrailerMismatchError` | `UND_ERR_TRAILER_MISMATCH` | trailers did not match specification |
| Error | Error Codes | Description |
| -----------------------------|--------------------------------------------|-------------------------------------------------------|
| `InvalidArgumentError` | `UND_ERR_INVALID_ARG` | passed an invalid argument. |
| `InvalidReturnValueError` | `UND_ERR_INVALID_RETURN_VALUE` | returned an invalid value. |
| `RequestAbortedError` | `UND_ERR_ABORTED` | the request has been aborted by the user |
| `ClientDestroyedError` | `UND_ERR_DESTROYED` | trying to use a destroyed client. |
| `ClientClosedError` | `UND_ERR_CLOSED` | trying to use a closed client. |
| `SocketError` | `UND_ERR_SOCKET` | there is an error with the socket. |
| `NotSupportedError` | `UND_ERR_NOT_SUPPORTED` | encountered unsupported functionality. |
| `RequestContentLengthError` | `UND_ERR_REQUEST_CONTENT_LENGTH_MISMATCH` | body does not match content-length header in request |
| `ResponseContentLengthError` | `UND_ERR_RESPONSE_CONTENT_LENGTH_MISMATCH`| body does not match content-length header in response * |
| `InformationalError` | `UND_ERR_INFO` | expected error with reason |
| `TrailerMismatchError` | `UND_ERR_TRAILER_MISMATCH` | trailers did not match specification |

\* The `UND_ERR_RESPONSE_CONTENT_LENGTH_MISMATCH` is returned when the received body is less than the value in the received header `Content-Length`.
If the server try to send a body larger than `Content-Length`, Undici closes the connection when the length is received and return an `UND_ERR_INFO`, to prevent a cache poisoning attack.
42 changes: 27 additions & 15 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const util = require('./core/util')
const Request = require('./core/request')
const Dispatcher = require('./dispatcher')
const {
ContentLengthMismatchError,
RequestContentLengthError,
ResponseContentLengthError,
TrailerMismatchError,
InvalidArgumentError,
RequestAbortedError,
Expand Down Expand Up @@ -450,6 +451,8 @@ class Parser {
this.headersSize = 0
this.headersMaxSize = client[kMaxHeadersSize]
this.shouldKeepAlive = false
this.bodySize = 0
this.contentLength = 0
this.paused = false
this.resume = this.resume.bind(this)
}
Expand Down Expand Up @@ -640,9 +643,6 @@ class Parser {

const request = client[kQueue][client[kRunningIdx]]
assert(request)

// TODO: Check for content-length mismatch from server?

assert(!this.upgrade)
assert(this.statusCode < 200)

Expand Down Expand Up @@ -689,23 +689,28 @@ class Parser {
}

assert(this.headers.length % 2 === 0)

this.headers = []
this.headersSize = 0

let keepAlive
let trailers
this.contentLength = 0

let looking = true
for (let n = 0; n < rawHeaders.length && looking; n += 2) {
for (let n = 0, looking = 0; n < rawHeaders.length && looking < 3; n += 2) {
const key = rawHeaders[n]
const val = rawHeaders[n + 1]

if (!keepAlive && key.length === 10 && key.toString().toLowerCase() === 'keep-alive') {
keepAlive = val
looking = !trailers
looking++
} else if (!trailers && key.length === 7 && key.toString().toLowerCase() === 'trailer') {
trailers = val
looking = !keepAlive
looking++
} else if (!this.contentLength && key.length === 14 && key.toString().toLowerCase() === 'content-length') {
// content-length here is always valid because lhttp validate it
this.contentLength = Number(val)
looking++
}
}

Expand Down Expand Up @@ -757,6 +762,8 @@ class Parser {
return -1
}

this.bodySize += buf.length

const request = client[kQueue][client[kRunningIdx]]
assert(request)

Expand Down Expand Up @@ -918,6 +925,11 @@ function onSocketError (err) {
}

function onSocketEnd () {
if (this[kParser].bodySize > 0 && this[kParser].contentLength > 0 &&
this[kParser].bodySize !== this[kParser].contentLength) {
util.destroy(this, new ResponseContentLengthError('other side closed'))
return
}
util.destroy(this, new SocketError('other side closed'))
}

Expand Down Expand Up @@ -1233,12 +1245,12 @@ function write (client, request) {

if (request.contentLength !== null && request.contentLength !== contentLength) {
if (client[kStrictContentLength]) {
request.onError(new ContentLengthMismatchError())
request.onError(new RequestContentLengthError())
assert(request.aborted)
return false
}

process.emitWarning(new ContentLengthMismatchError())
process.emitWarning(new RequestContentLengthError())
}

const socket = client[kSocket]
Expand Down Expand Up @@ -1305,7 +1317,7 @@ function write (client, request) {

if (!body) {
if (contentLength === 0) {
socket.write(`${header}content-length: ${contentLength}\r\n\r\n\r\n`, 'ascii')
socket.write(`${header}content-length: 0\r\n\r\n\r\n`, 'ascii')
} else {
assert(contentLength === null, 'no body must not have content length')
socket.write(`${header}\r\n`, 'ascii')
Expand Down Expand Up @@ -1344,11 +1356,11 @@ function write (client, request) {
// We should defer writing chunks.
if (contentLength !== null && bytesWritten + len > contentLength) {
if (client[kStrictContentLength]) {
util.destroy(socket, new ContentLengthMismatchError())
util.destroy(socket, new RequestContentLengthError())
return
}

process.emitWarning(new ContentLengthMismatchError())
process.emitWarning(new RequestContentLengthError())
}

if (bytesWritten === 0) {
Expand Down Expand Up @@ -1398,9 +1410,9 @@ function write (client, request) {

if (!err && contentLength !== null && bytesWritten !== contentLength) {
if (client[kStrictContentLength]) {
err = new ContentLengthMismatchError()
err = new RequestContentLengthError()
} else {
process.emitWarning(new ContentLengthMismatchError())
process.emitWarning(new RequestContentLengthError())
}
}

Expand Down
21 changes: 16 additions & 5 deletions lib/core/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,23 @@ class InformationalError extends UndiciError {
}
}

class ContentLengthMismatchError extends UndiciError {
class RequestContentLengthError extends UndiciError {
constructor (message) {
super(message)
Error.captureStackTrace(this, ContentLengthMismatchError)
this.name = 'ContentLengthMismatchError'
Error.captureStackTrace(this, RequestContentLengthError)
this.name = 'RequestContentLengthError'
this.message = message || 'Request body length does not match content-length header'
this.code = 'UND_ERR_CONTENT_LENGTH_MISMATCH'
this.code = 'UND_ERR_REQUEST_CONTENT_LENGTH_MISMATCH'
}
}

class ResponseContentLengthError extends UndiciError {
constructor (message) {
super(message)
Error.captureStackTrace(this, ResponseContentLengthError)
this.name = 'ResponseContentLengthError'
this.message = message || 'Response body length does not match content-length header'
this.code = 'UND_ERR_RESPONSE_CONTENT_LENGTH_MISMATCH'
}
}

Expand Down Expand Up @@ -153,7 +163,8 @@ module.exports = {
HeadersTimeoutError,
HeadersOverflowError,
BodyTimeoutError,
ContentLengthMismatchError,
RequestContentLengthError,
ResponseContentLengthError,
ConnectTimeoutError,
TrailerMismatchError,
InvalidArgumentError,
Expand Down
Loading

0 comments on commit ffc97e8

Please sign in to comment.