Skip to content

Commit

Permalink
Add exception for HTTP clients that set a content-type when there is …
Browse files Browse the repository at this point in the history
…no body (#2599)

* fix(xrpc-server): add exception for HTTP clients that set a content-type when there is no body

* feat(xrpc-server): allow empty body on endpoints not expecting any input

Co-authored-by: Devin Ivy <devinivy@gmail.com>

* fix(xrpc-server): properly test empty body requests

* tidy

---------

Co-authored-by: Devin Ivy <devinivy@gmail.com>
  • Loading branch information
matthieusieben and devinivy authored Jun 24, 2024
1 parent aed98cc commit fc10881
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 20 deletions.
29 changes: 10 additions & 19 deletions packages/xrpc-server/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,17 @@ export function validateInput(
lexicons: Lexicons,
): HandlerInput | undefined {
// request expectation
const reqHasBody = hasBody(req)
if (reqHasBody && (def.type !== 'procedure' || !def.input)) {

const bodyPresence = getBodyPresence(req)
if (bodyPresence === 'present' && (def.type !== 'procedure' || !def.input)) {
throw new InvalidRequestError(
`A request body was provided when none was expected`,
)
}
if (def.type === 'query') {
return
}
if (!reqHasBody && def.input) {
if (bodyPresence === 'missing' && def.input) {
throw new InvalidRequestError(
`A request body is expected but none was provided`,
)
Expand Down Expand Up @@ -214,23 +215,13 @@ function isValidEncoding(possibleStr: string, value: string) {
return possible.includes(normalized)
}

function parseContentLength(value: string): number {
if (/^\s*\d+\s*$/.test(value)) return Number(value)
throw new InvalidRequestError('invalid content-length header')
}

function hasBody(req: express.Request): boolean {
if (req.headers['content-length']) {
const contentLength = parseContentLength(req.headers['content-length'])
if (contentLength > 0) return true
// A content-length of 0 is still a body if there is a content-type (e.g.
// an empty text file)
if (req.headers['content-type']) return true
}

if (req.headers['transfer-encoding']) return true
type BodyPresence = 'missing' | 'empty' | 'present'

return false
function getBodyPresence(req: express.Request): BodyPresence {
if (req.headers['transfer-encoding'] != null) return 'present'
if (req.headers['content-length'] === '0') return 'empty'
if (req.headers['content-length'] != null) return 'present'
return 'missing'
}

export function processBodyAsBytes(req: express.Request): Promise<Uint8Array> {
Expand Down
13 changes: 12 additions & 1 deletion packages/xrpc-server/tests/bodies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ describe('Bodies', () => {
expect(res1.data.bar).toBe(123)

await expect(client.call('io.example.validationTest', {})).rejects.toThrow(
`A request body is expected but none was provided`,
'Request encoding (Content-Type) required but not provided',
)
await expect(
client.call('io.example.validationTest', {}, {}),
Expand Down Expand Up @@ -229,6 +229,17 @@ describe('Bodies', () => {
expect(compressed.cid).toEqual(expectedCid.toString())
})

it('supports empty payload', async () => {
const expectedCid = await cidForCbor(new Uint8Array(0))

// Using "undefined" as body to avoid encoding as lexicon { $bytes: "<base64>" }
const result = await client.call('io.example.blobTest', {}, undefined, {
encoding: 'text/plain',
})

expect(result.data.cid).toEqual(expectedCid.toString())
})

it('supports max blob size (based on content-length)', async () => {
const bytes = randomBytes(BLOB_LIMIT + 1)

Expand Down

0 comments on commit fc10881

Please sign in to comment.