Skip to content

Commit

Permalink
fix: set "X-Request-Id" header in XMLHttpRequest only in Node.js (#397)
Browse files Browse the repository at this point in the history
Co-authored-by: avivasyuta <[email protected]>
Co-authored-by: Artem Zakharchenko <[email protected]>
  • Loading branch information
3 people authored Aug 28, 2023
1 parent 7df9776 commit 0adedc5
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 21 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@
"@open-draft/logger": "^0.3.0",
"@open-draft/until": "^2.0.0",
"headers-polyfill": "^3.1.0",
"is-node-process": "^1.2.0",
"outvariant": "^1.2.1",
"strict-event-emitter": "^0.5.0"
},
Expand Down
2 changes: 2 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 8 additions & 5 deletions src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { invariant } from 'outvariant'
import { headersToString } from 'headers-polyfill'
import { isNodeProcess } from 'is-node-process'
import type { Logger } from '@open-draft/logger'
import { concatArrayBuffer } from './utils/concatArrayBuffer'
import { createEvent } from './utils/createEvent'
Expand All @@ -15,14 +16,15 @@ import { uuidv4 } from '../../utils/uuid'
import { createResponse } from './utils/createResponse'

const IS_MOCKED_RESPONSE = Symbol('isMockedResponse')
const IS_NODE = isNodeProcess()

/**
* An `XMLHttpRequest` instance controller that allows us
* to handle any given request instance (e.g. responding to it).
*/
export class XMLHttpRequestController {
public request: XMLHttpRequest
public requestId?: string
public requestId: string
public onRequest?: (
this: XMLHttpRequestController,
args: {
Expand All @@ -49,6 +51,7 @@ export class XMLHttpRequestController {

constructor(readonly initialRequest: XMLHttpRequest, public logger: Logger) {
this.events = new Map()
this.requestId = uuidv4()
this.requestHeaders = new Headers()
this.responseBuffer = new Uint8Array()

Expand Down Expand Up @@ -80,8 +83,6 @@ export class XMLHttpRequestController {
case 'open': {
const [method, url] = args as [string, string | undefined]

this.requestId = uuidv4()

if (typeof url === 'undefined') {
this.method = 'GET'
this.url = toAbsoluteUrl(method)
Expand Down Expand Up @@ -171,15 +172,17 @@ export class XMLHttpRequestController {
)

/**
* @note Set the intercepted request ID on the original request
* @note Set the intercepted request ID on the original request in Node.js
* so that if it triggers any other interceptors, they don't attempt
* to process it once again.
*
* For instance, XMLHttpRequest is often implemented via "http.ClientRequest"
* and we don't want for both XHR and ClientRequest interceptors to
* handle the same request at the same time (e.g. emit the "response" event twice).
*/
this.request.setRequestHeader('X-Request-Id', this.requestId!)
if (IS_NODE) {
this.request.setRequestHeader('X-Request-Id', this.requestId!)
}

return invoke()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,34 @@
import { HttpServer } from '@open-draft/test-server/http'
import { RequestHandler } from 'express-serve-static-core'
import { test, expect } from '../../../playwright.extend'
import { invariant } from 'outvariant'

const httpServer = new HttpServer((app) => {
const strictCorsMiddleware: RequestHandler = (req, res, next) => {
invariant(
!req.header('x-request-id'),
'Found unexpected "X-Request-Id" request header in the browser for "%s %s"',
req.method,
req.url
)

res
.set('Access-Control-Allow-Origin', req.headers.origin)
.set('Access-Control-Allow-Methods', 'GET, POST')
.set('Access-Control-Allow-Headers', [
'content-type',
'x-request-id',
'x-request-header',
])
.set('Access-Control-Allow-Headers', ['content-type', 'x-request-header'])
.set('Access-Control-Allow-Credentials', 'true')
return next()
}

app.use(strictCorsMiddleware)

const requestHandler: RequestHandler = (req, res) => {
res.status(200).send('user-body')
}

app.options('/user', strictCorsMiddleware, (req, res) =>
res.status(200).end()
)
app.get('/user', strictCorsMiddleware, requestHandler)
app.post('/user', strictCorsMiddleware, requestHandler)
app.options('/user', (req, res) => res.status(200).end())
app.get('/user', requestHandler)
app.post('/user', requestHandler)
})

test.beforeAll(async () => {
Expand Down Expand Up @@ -135,31 +139,37 @@ test('sets "credentials" to "same-origin" on isomorphic request when "withCreden
expect(request.credentials).toBe('same-origin')
})

test('ignores the body for HEAD requests', async ({ loadExample, callXMLHttpRequest}) => {
test('ignores the body for HEAD requests', async ({
loadExample,
callXMLHttpRequest,
}) => {
await loadExample(require.resolve('./XMLHttpRequest.browser.runtime.js'))

const url = httpServer.http.url('/user')
const call = callXMLHttpRequest({
method: 'HEAD',
url,
body: "test"
});
body: 'test',
})

await expect(call).resolves.not.toThrowError()

const [request] = await call
expect(request.body).toBe(null)
})

test('ignores the body for GET requests', async ({ loadExample, callXMLHttpRequest}) => {
test('ignores the body for GET requests', async ({
loadExample,
callXMLHttpRequest,
}) => {
await loadExample(require.resolve('./XMLHttpRequest.browser.runtime.js'))

const url = httpServer.http.url('/user')
const call = callXMLHttpRequest({
method: 'GET',
url,
body: "test"
});
body: 'test',
})

await expect(call).resolves.not.toThrowError()

Expand Down

0 comments on commit 0adedc5

Please sign in to comment.