fix: decode urls in prerequest (#28427)

cli/CHANGELOG.md

@@ -7,6 +7,7 @@ _Released 12/5/2023 (PENDING)_
 
 - Fixed an issue where pages or downloads opened in a new tab were missing basic auth headers. Fixes [#28350](https://github.com/cypress-io/cypress/issues/28350).
 - Fixed an issue where request logging would default the `message` to the `args` of the currently running command even though those `args` would not apply to the request log and are not displayed. If the `args` are sufficiently large (e.g. when running the `cy.task` from the [code-coverage](https://github.com/cypress-io/code-coverage/) plugin) there could be performance/memory implications. Addressed in [#28411](https://github.com/cypress-io/cypress/pull/28411).
+- Fixed issue where some URLs would timeout in pre-request correlation. Addressed in [#28427](https://github.com/cypress-io/cypress/pull/28427).
 
 **Misc:**
 

packages/proxy/lib/http/util/prerequests.ts

@@ -148,7 +148,7 @@ export class PreRequests {
 
  addPending (browserPreRequest: BrowserPreRequest) {
  metrics.browserPreRequestsReceived++
- const key = `${browserPreRequest.method}-${browserPreRequest.url}`
+ const key = `${browserPreRequest.method}-${decodeURI(browserPreRequest.url)}`
  const pendingRequest = this.pendingRequests.shift(key)
 
  if (pendingRequest) {
@@ -193,7 +193,7 @@ export class PreRequests {
  }
 
  addPendingUrlWithoutPreRequest (url: string) {
- const key = `GET-${url}`
+ const key = `GET-${decodeURI(url)}`
  const pendingRequest = this.pendingRequests.shift(key)
 
  if (pendingRequest) {
@@ -236,7 +236,7 @@ export class PreRequests {
  const proxyRequestReceivedTimestamp = performance.now() + performance.timeOrigin
 
  metrics.proxyRequestsReceived++
- const key = `${req.method}-${req.proxiedUrl}`
+ const key = `${req.method}-${decodeURI(req.proxiedUrl)}`
  const pendingPreRequest = this.pendingPreRequests.shift(key)
 
  if (pendingPreRequest) {

packages/proxy/test/unit/http/util/prerequests.spec.ts

@@ -275,4 +275,25 @@ describe('http/util/prerequests', () => {
 
  expect(callbackCalled).to.be.true
  })
+
+ it('decodes the proxied url', () => {
+ preRequests.get({ proxiedUrl: 'foo%7Cbar', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, () => {})
+
+ expect(preRequests.pendingRequests.length).to.eq(1)
+ expect(preRequests.pendingRequests.shift('GET-foo|bar')).not.to.be.undefined
+ })
+
+ it('decodes the pending url without pre-request', () => {
+ preRequests.addPendingUrlWithoutPreRequest('foo%7Cbar')
+
+ expect(preRequests.pendingUrlsWithoutPreRequests.length).to.eq(1)
+ expect(preRequests.pendingUrlsWithoutPreRequests.shift('GET-foo|bar')).not.to.be.undefined
+ })
+
+ it('decodes pending url', () => {
+ preRequests.addPending({ requestId: '1234', url: 'foo%7Cbar', method: 'GET' } as BrowserPreRequest)
+
+ expect(preRequests.pendingPreRequests.length).to.eq(1)
+ expect(preRequests.pendingPreRequests.shift('GET-foo|bar')).not.to.be.undefined
+ })
 })

packages/server/test/integration/http_requests_spec.js

@@ -988,7 +988,9 @@ describe('Routes', () => {
 
  context('basic request with correlation', () => {
  beforeEach(function () {
- return this.setup('http://www.github.com', undefined, undefined, true)
+ return this.setup('http://www.github.com', undefined, undefined, true).then(() => {
+ this.networkProxy.setPreRequestTimeout(2000)
+ })
  })
 
  it('properly correlates when CDP failures come first', function () {
@@ -1087,6 +1089,147 @@ describe('Routes', () => {
  })
  })
  })
+
+ it('properly correlates when request has | character', function () {
+ this.timeout(1500)
+
+ nock(this.server.remoteStates.current().origin)
+ .get('/?foo=bar|baz')
+ .reply(200, 'hello from bar!', {
+ 'Content-Type': 'text/html',
+ })
+
+ const requestPromise = this.rp({
+ url: 'http://www.github.com/?foo=bar|baz',
+ headers: {
+ 'Accept-Encoding': 'identity',
+ },
+ })
+
+ this.networkProxy.addPendingBrowserPreRequest({
+ requestId: '1',
+ method: 'GET',
+ url: 'http://www.github.com/?foo=bar|baz',
+ })
+
+ return requestPromise.then((res) => {
+ expect(res.statusCode).to.eq(200)
+
+ expect(res.body).to.include('hello from bar!')
+ })
+ })
+
+ it('properly correlates when request has | character with addPendingUrlWithoutPreRequest', function () {
+ this.timeout(1500)
+
+ nock(this.server.remoteStates.current().origin)
+ .get('/?foo=bar|baz')
+ .reply(200, 'hello from bar!', {
+ 'Content-Type': 'text/html',
+ })
+
+ const requestPromise = this.rp({
+ url: 'http://www.github.com/?foo=bar|baz',
+ headers: {
+ 'Accept-Encoding': 'identity',
+ },
+ })
+
+ this.networkProxy.addPendingUrlWithoutPreRequest('http://www.github.com/?foo=bar|baz')
+
+ return requestPromise.then((res) => {
+ expect(res.statusCode).to.eq(200)
+
+ expect(res.body).to.include('hello from bar!')
+ })
+ })
+
+ it('properly correlates when request has encoded | character', function () {
+ this.timeout(1500)
+
+ nock(this.server.remoteStates.current().origin)
+ .get('/?foo=bar%7Cbaz')
+ .reply(200, 'hello from bar!', {
+ 'Content-Type': 'text/html',
+ })
+
+ const requestPromise = this.rp({
+ url: 'http://www.github.com/?foo=bar%7Cbaz',
+ headers: {
+ 'Accept-Encoding': 'identity',
+ },
+ })
+
+ this.networkProxy.addPendingBrowserPreRequest({
+ requestId: '1',
+ method: 'GET',
+ url: 'http://www.github.com/?foo=bar%7Cbaz',
+ })
+
+ return requestPromise.then((res) => {
+ expect(res.statusCode).to.eq(200)
+
+ expect(res.body).to.include('hello from bar!')
+ })
+ })
+
+ it('properly correlates when request has encoded " " (space) character', function () {
+ this.timeout(1500)
+
+ nock(this.server.remoteStates.current().origin)
+ .get('/?foo=bar%20baz')
+ .reply(200, 'hello from bar!', {
+ 'Content-Type': 'text/html',
+ })
+
+ const requestPromise = this.rp({
+ url: 'http://www.github.com/?foo=bar%20baz',
+ headers: {
+ 'Accept-Encoding': 'identity',
+ },
+ })
+
+ this.networkProxy.addPendingBrowserPreRequest({
+ requestId: '1',
+ method: 'GET',
+ url: 'http://www.github.com/?foo=bar%20baz',
+ })
+
+ return requestPromise.then((res) => {
+ expect(res.statusCode).to.eq(200)
+
+ expect(res.body).to.include('hello from bar!')
+ })
+ })
+
+ it('properly correlates when request has encoded " character', function () {
+ this.timeout(1500)
+
+ nock(this.server.remoteStates.current().origin)
+ .get('/?foo=bar%22')
+ .reply(200, 'hello from bar!', {
+ 'Content-Type': 'text/html',
+ })
+
+ const requestPromise = this.rp({
+ url: 'http://www.github.com/?foo=bar%22',
+ headers: {
+ 'Accept-Encoding': 'identity',
+ },
+ })
+
+ this.networkProxy.addPendingBrowserPreRequest({
+ requestId: '1',
+ method: 'GET',
+ url: 'http://www.github.com/?foo=bar%22',
+ })
+
+ return requestPromise.then((res) => {
+ expect(res.statusCode).to.eq(200)
+
+ expect(res.body).to.include('hello from bar!')
+ })
+ })
  })
 
  context('gzip', () => {