hapijs / hapijs/pez

multipart validation doesn't comply with new fetch api

Open
#53 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
JavaScript
Stars
34
Forks
19
PR merge metrics
No merged PRs in 30d

Description

### Runtime

node.js

### Runtime version

18+

### Module version

21.3.3

### Last module version without issue

_No response_

### Used with

_No response_

### Any other relevant information

_No response_

### What are you trying to achieve or the steps to reproduce?

Set multipart payload validation:

```
payload: {
// 2MB
maxBytes: 2097152,
maxParts: 2,
multipart: {
output: "stream"
},
parse: true
},
```

Pass a payload larger than limit via `fetch` from web

```
try {
const response = await fetch(`${HOST}:${PORT}/image`, {
body: image,
headers: {
Authorization: `Bearer ${token}`
},
method: "POST"
})

if (!response.ok) {
if(response.status === 413) {
console.error("Image too large!")
}
}
} catch(error) {
console.error("Generic error")
console.error(error)
}
```

Expect to handle 413 when `response.status === 413`

### What was the result you got?

`Generic error`

### What result did you expect?

`Image too large!`

The problem here is actually in `hapijs/pez` and the way it handles `maxBytes` - and other multipart errors

`pez` hasn't been touched in a year and a half though and has near zero activity, but it is officially part of Hapi so I raised it here where activity is actually happening :smile:

[On line 136 of `lib/index.js` in `hapijs/pez`:
](https://github.com/hapijs/pez/blob/60b1270418efea457584d4c0007fd379f571036d/lib/index.js#L136)

```
const onReqData = (data) => {

this._bytesCount += Buffer.byteLength(data);

if (this._bytesCount > this._maxBytes) {
finish(Boom.entityTooLarge('Maximum size exceeded'));
}
};
```

This works but it happens before the actual Hapi validation occurs. I understand that multipart is being parsed before but this **doesn't** work with the new `fetch` API

From [MDN Docs](https://developer.mozilla.org/en-US/docs/Web/API/fetch):

> A fetch() promise only rejects when the request fails, for example, because of a badly-formed request URL or a network error. A fetch() promise does not reject if the server responds with HTTP status codes that indicate errors (404, 504, etc.). Instead, a then() handler must check the [Response.ok](https://developer.mozilla.org/en-US/docs/Web/API/Response/ok) and/or [Response.status](https://developer.mozilla.org/en-US/docs/Web/API/Response/status) properties.

Every other validation in Hapi - say within `failAction` - behaves exactly as the `fetch` API expects. Request fails with `!response.ok` and response contains both `response.statusText` and `response.status`

Because of how early `hapijs/pez` throws the error the `fetch` API actually interprets it as a lower level error. `fetch` actually `throws` and never gets to the `!response.ok` part

That would actually be okay as we can catch it, if it wasn't that that way we completely lose `statusText` and `status` and those are... essential for actually displaying a formatted error say in the UI. This example has `console.error` for simplicity

We use `hapi` in production **everywhere** for our projects - I really like `hapi` :blush: - and we just ran into this one

I know `hapijs/pez` doesn't get worked on a lot but this is a pretty major issue for UI/UX

I can help with a PR if you want me to - maybe a little guidance on best practices?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.