From 457a2f83e7bdeacd24da09d54240a1fb7f9afa8e Mon Sep 17 00:00:00 2001 From: naman-bruno Date: Wed, 3 Sep 2025 17:52:12 +0530 Subject: [PATCH] fix: Bruno GUI hangs on 308 redirect (#5445) * fix: 308 redirect --- .github/workflows/tests.yml | 5 ++ .../src/runner/run-single-request.js | 2 + .../bruno-cli/src/utils/axios-instance.js | 23 +++++++ .../src/ipc/network/axios-instance.js | 36 +++++++++++ .../bruno-electron/src/ipc/network/index.js | 2 + .../collection/environments/Local.bru | 1 + .../collection/environments/Prod.bru | 1 + ...t Multipart Redirect Consumed FormData.bru | 45 +++++++++++++ ...est Multipart Redirect Multiple Fields.bru | 48 ++++++++++++++ .../redirects/Test Multipart Redirect.bru | 41 ++++++++++++ packages/bruno-tests/src/index.js | 2 + packages/bruno-tests/src/redirect/index.js | 64 +++++++++++++++++++ 12 files changed, 270 insertions(+) create mode 100644 packages/bruno-tests/collection/redirects/Test Multipart Redirect Consumed FormData.bru create mode 100644 packages/bruno-tests/collection/redirects/Test Multipart Redirect Multiple Fields.bru create mode 100644 packages/bruno-tests/collection/redirects/Test Multipart Redirect.bru create mode 100644 packages/bruno-tests/src/redirect/index.js diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2a829098e..4bf2d753c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -83,6 +83,11 @@ jobs: npm run build --workspace=packages/bruno-requests npm run build --workspace=packages/bruno-filestore + - name: Run Local Testbench + run: | + npm start --workspace=packages/bruno-tests & + sleep 5 + - name: Run tests run: | cd packages/bruno-tests/collection diff --git a/packages/bruno-cli/src/runner/run-single-request.js b/packages/bruno-cli/src/runner/run-single-request.js index a682d4814..9b9ca8c74 100644 --- a/packages/bruno-cli/src/runner/run-single-request.js +++ b/packages/bruno-cli/src/runner/run-single-request.js @@ -347,6 +347,8 @@ const runSingleRequest = async function ( if (contentTypeHeader && request.headers[contentTypeHeader] === 'multipart/form-data') { if (!(request?.data instanceof FormData)) { + request._originalMultipartData = request.data; + request.collectionPath = collectionPath; let form = createFormData(request.data, collectionPath); request.data = form; extend(request.headers, form.getHeaders()); diff --git a/packages/bruno-cli/src/utils/axios-instance.js b/packages/bruno-cli/src/utils/axios-instance.js index 1d0e7ccd9..4bba1af73 100644 --- a/packages/bruno-cli/src/utils/axios-instance.js +++ b/packages/bruno-cli/src/utils/axios-instance.js @@ -1,6 +1,7 @@ const axios = require('axios'); const { CLI_VERSION } = require('../constants'); const { addCookieToJar, getCookieStringForUrl } = require('./cookies'); +const { createFormData } = require('./form-data'); const redirectResponseCodes = [301, 302, 303, 307, 308]; const METHOD_CHANGING_REDIRECTS = [301, 302, 303]; @@ -38,6 +39,28 @@ const createRedirectConfig = (error, redirectUrl) => { delete requestConfig.headers['Content-Length']; delete requestConfig.headers['content-type']; delete requestConfig.headers['Content-Type']; + } else { + // For 307, 308 and other status codes: preserve method and body + if (requestConfig.data && typeof requestConfig.data === 'object' && + requestConfig.data.constructor && requestConfig.data.constructor.name === 'FormData') { + + const formData = requestConfig.data; + if (formData._released || (formData._streams && formData._streams.length === 0)) { + if (error.config._originalMultipartData && error.config.collectionPath) { + const recreatedForm = createFormData(error.config._originalMultipartData, error.config.collectionPath); + requestConfig.data = recreatedForm; + const formHeaders = recreatedForm.getHeaders(); + Object.assign(requestConfig.headers, formHeaders); + + // preserve the original data for potential future redirects + requestConfig._originalMultipartData = error.config._originalMultipartData; + requestConfig.collectionPath = error.config.collectionPath; + } + } else { + requestConfig._originalMultipartData = error.config._originalMultipartData; + requestConfig.collectionPath = error.config.collectionPath; + } + } } return requestConfig; diff --git a/packages/bruno-electron/src/ipc/network/axios-instance.js b/packages/bruno-electron/src/ipc/network/axios-instance.js index 3b3cc2b14..1dd180cee 100644 --- a/packages/bruno-electron/src/ipc/network/axios-instance.js +++ b/packages/bruno-electron/src/ipc/network/axios-instance.js @@ -7,6 +7,7 @@ const { setupProxyAgents } = require('../../utils/proxy-util'); const { addCookieToJar, getCookieStringForUrl } = require('../../utils/cookies'); const { preferencesUtil } = require('../../store/preferences'); const { safeStringifyJSON } = require('../../utils/common'); +const { createFormData } = require('../../utils/form-data'); const LOCAL_IPV6 = '::1'; const LOCAL_IPV4 = '127.0.0.1'; @@ -328,6 +329,41 @@ function makeAxiosInstance({ type: 'info', message: `Changed method from ${originalMethod.toUpperCase()} to GET for ${statusCode} redirect and removed request body`, }); + } else { + // For 307, 308 and other status codes: preserve method and body + if (requestConfig.data && typeof requestConfig.data === 'object' && + requestConfig.data.constructor && requestConfig.data.constructor.name === 'FormData') { + + const formData = requestConfig.data; + if (formData._released || (formData._streams && formData._streams.length === 0)) { + if (error.config._originalMultipartData && error.config.collectionPath) { + timeline.push({ + timestamp: new Date(), + type: 'info', + message: `Recreating consumed FormData for ${statusCode} redirect`, + }); + + const recreatedForm = createFormData(error.config._originalMultipartData, error.config.collectionPath); + requestConfig.data = recreatedForm; + + const formHeaders = recreatedForm.getHeaders(); + Object.assign(requestConfig.headers, formHeaders); + + // preserve the original data for potential future redirects + requestConfig._originalMultipartData = error.config._originalMultipartData; + requestConfig.collectionPath = error.config.collectionPath; + } else { + timeline.push({ + timestamp: new Date(), + type: 'info', + message: `FormData consumed but no original data available for ${statusCode} redirect`, + }); + } + } else { + requestConfig._originalMultipartData = error.config._originalMultipartData; + requestConfig.collectionPath = error.config.collectionPath; + } + } } if (preferencesUtil.shouldSendCookies()) { diff --git a/packages/bruno-electron/src/ipc/network/index.js b/packages/bruno-electron/src/ipc/network/index.js index bf22f4f66..5ae010bc2 100644 --- a/packages/bruno-electron/src/ipc/network/index.js +++ b/packages/bruno-electron/src/ipc/network/index.js @@ -402,6 +402,8 @@ const registerNetworkIpc = (mainWindow) => { if (request.headers['content-type'] === 'multipart/form-data') { if (!(request.data instanceof FormData)) { + request._originalMultipartData = request.data; + request.collectionPath = collectionPath; let form = createFormData(request.data, collectionPath); request.data = form; extend(request.headers, form.getHeaders()); diff --git a/packages/bruno-tests/collection/environments/Local.bru b/packages/bruno-tests/collection/environments/Local.bru index a7ac3f541..9980c994e 100644 --- a/packages/bruno-tests/collection/environments/Local.bru +++ b/packages/bruno-tests/collection/environments/Local.bru @@ -1,5 +1,6 @@ vars { host: http://localhost:8080 + localhost: http://localhost:8081 httpfaker: https://www.httpfaker.org bearer_auth_token: your_secret_token basic_auth_password: della diff --git a/packages/bruno-tests/collection/environments/Prod.bru b/packages/bruno-tests/collection/environments/Prod.bru index f33c1bb05..92ba5fc76 100644 --- a/packages/bruno-tests/collection/environments/Prod.bru +++ b/packages/bruno-tests/collection/environments/Prod.bru @@ -1,5 +1,6 @@ vars { host: https://testbench-sanity.usebruno.com + localhost: http://localhost:8081 httpfaker: https://www.httpfaker.org bearer_auth_token: your_secret_token basic_auth_password: della diff --git a/packages/bruno-tests/collection/redirects/Test Multipart Redirect Consumed FormData.bru b/packages/bruno-tests/collection/redirects/Test Multipart Redirect Consumed FormData.bru new file mode 100644 index 000000000..1b00a438a --- /dev/null +++ b/packages/bruno-tests/collection/redirects/Test Multipart Redirect Consumed FormData.bru @@ -0,0 +1,45 @@ +meta { + name: Test Multipart Redirect Consumed FormData + type: http + seq: 7 +} + +post { + url: {{localhost}}/api/redirect/multipart-redirect-source + body: multipartForm + auth: none +} + +body:multipart-form { + consumed-field: consumed-value +} + +assert { + res.status: 200 +} + +tests { + test("should handle consumed FormData recreation during 308 redirect", function() { + const data = res.getBody(); + expect(data).to.be.an('object'); + expect(data.status).to.equal('success'); + expect(data.method).to.equal('POST'); + }); + + test("should preserve POST method when FormData is consumed and recreated", function() { + const data = res.getBody(); + expect(data.method).to.equal('POST'); + }); + + test("should receive form data after FormData recreation", function() { + const data = res.getBody(); + expect(data.body).to.have.property('consumed-field'); + expect(data.body['consumed-field']).to.equal('consumed-value'); + }); + + test("should maintain proper content-type after FormData recreation", function() { + const data = res.getBody(); + expect(data.headers).to.have.property('content-type'); + expect(data.headers['content-type']).to.include('multipart/form-data'); + }); +} diff --git a/packages/bruno-tests/collection/redirects/Test Multipart Redirect Multiple Fields.bru b/packages/bruno-tests/collection/redirects/Test Multipart Redirect Multiple Fields.bru new file mode 100644 index 000000000..29da01222 --- /dev/null +++ b/packages/bruno-tests/collection/redirects/Test Multipart Redirect Multiple Fields.bru @@ -0,0 +1,48 @@ +meta { + name: Test Multipart Redirect Multiple Fields + type: http + seq: 5 +} + +post { + url: {{localhost}}/api/redirect/multipart-redirect-source + body: multipartForm + auth: none +} + +body:multipart-form { + field1: value1 + field2: value2 + field3: value3 +} + +assert { + res.status: 200 +} + +tests { + test("should successfully redirect complex multipart form data with 308", function() { + const data = res.getBody(); + expect(data).to.be.an('object'); + expect(data.status).to.equal('success'); + expect(data.method).to.equal('POST'); + }); + + test("should preserve POST method during redirect", function() { + const data = res.getBody(); + expect(data.method).to.equal('POST'); + }); + + test("should receive all text fields at target endpoint", function() { + const data = res.getBody(); + expect(data.body).to.have.property('field1'); + expect(data.body).to.have.property('field2'); + expect(data.body).to.have.property('field3'); + }); + + test("should maintain content-type header during redirect", function() { + const data = res.getBody(); + expect(data.headers).to.have.property('content-type'); + expect(data.headers['content-type']).to.include('multipart/form-data'); + }); +} diff --git a/packages/bruno-tests/collection/redirects/Test Multipart Redirect.bru b/packages/bruno-tests/collection/redirects/Test Multipart Redirect.bru new file mode 100644 index 000000000..099ed4ba6 --- /dev/null +++ b/packages/bruno-tests/collection/redirects/Test Multipart Redirect.bru @@ -0,0 +1,41 @@ +meta { + name: Test Multipart Redirect + type: http + seq: 3 +} + +post { + url: {{localhost}}/api/redirect/multipart-redirect-source + body: multipartForm + auth: none +} + +body:multipart-form { + test-field: test-value +} + +assert { + res.status: 200 +} + +tests { + test("should successfully redirect multipart form data with 308", function() { + const data = res.getBody(); + expect(data).to.be.an('object'); + expect(data.status).to.equal('success'); + expect(data.method).to.equal('POST'); + expect(data.body).to.be.an('object'); + expect(data.body['test-field']).to.equal('test-value'); + }); + + test("should preserve POST method during redirect", function() { + const data = res.getBody(); + expect(data.method).to.equal('POST'); + }); + + test("should receive form data at target endpoint", function() { + const data = res.getBody(); + expect(data.body).to.have.property('test-field'); + expect(data.body['test-field']).to.equal('test-value'); + }); +} diff --git a/packages/bruno-tests/src/index.js b/packages/bruno-tests/src/index.js index 735bd929b..2e39f7a9c 100644 --- a/packages/bruno-tests/src/index.js +++ b/packages/bruno-tests/src/index.js @@ -6,6 +6,7 @@ const authRouter = require('./auth'); const echoRouter = require('./echo'); const xmlParser = require('./utils/xmlParser'); const multipartRouter = require('./multipart'); +const redirectRouter = require('./redirect'); const app = new express(); const port = process.env.PORT || 8081; @@ -28,6 +29,7 @@ formDataParser.init(app, express); app.use('/api/auth', authRouter); app.use('/api/echo', echoRouter); app.use('/api/multipart', multipartRouter); +app.use('/api/redirect', redirectRouter); app.get('/ping', function (req, res) { return res.send('pong'); diff --git a/packages/bruno-tests/src/redirect/index.js b/packages/bruno-tests/src/redirect/index.js new file mode 100644 index 000000000..4573fdf74 --- /dev/null +++ b/packages/bruno-tests/src/redirect/index.js @@ -0,0 +1,64 @@ +const express = require('express'); +const formDataParser = require('../multipart/form-data-parser'); +const router = express.Router(); + +const parseMultipartFormData = (req) => { + if (req.headers['content-type'] && req.headers['content-type'].includes('multipart/form-data')) { + try { + const parts = formDataParser.parse(req); + const parsedBody = {}; + const files = []; + + parts.forEach(part => { + if (part.filename) { + files.push({ + fieldname: part.name, + originalname: part.filename, + mimetype: part.contentType, + size: part.value ? part.value.length : 0 + }); + } else { + parsedBody[part.name] = part.value; + } + }); + + return { body: parsedBody, files }; + } catch (error) { + console.error('Error parsing multipart form data:', error); + return { body: {}, files: [] }; + } + } + return { body: req.body, files: [] }; +}; + +router.post('/multipart-redirect-source', function (req, res) { + console.log('Multipart redirect source endpoint hit'); + console.log('Method:', req.method); + console.log('Headers:', req.headers); + + const { body, files } = parseMultipartFormData(req); + console.log('Parsed Body:', body); + console.log('Files:', files); + + res.status(308).location('/api/redirect/multipart-redirect-target').send('Permanently moved'); +}); + +router.post('/multipart-redirect-target', function (req, res) { + console.log('Multipart redirect target endpoint hit'); + console.log('Method:', req.method); + console.log('Headers:', req.headers); + + const { body, files } = parseMultipartFormData(req); + console.log('Parsed Body:', body); + console.log('Files:', files); + + res.json({ + status: 'success', + method: req.method, + body: body, + files: files, + headers: req.headers + }); +}); + +module.exports = router; \ No newline at end of file