Skip to content

Allow passthrough redirects #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions example.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ const proxy = require('.')

async function startOrigin () {
const origin = Fastify()

origin.get('/', async (request, reply) => {
return 'this is root'
})

origin.get('/redirect', async (request, reply) => {
return reply.redirect(302, 'https://fastify.io')
})

origin.get('/a', async (request, reply) => {
return 'this is a'
})
Expand Down
8 changes: 6 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
'use strict'

const From = require('fastify-reply-from')
const WebSocket = require('ws')

const httpMethods = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT', 'OPTIONS']
const urlPattern = /^https?:\/\//

function liftErrorCode (code) {
if (typeof code !== 'number') {
Expand Down Expand Up @@ -31,6 +31,10 @@ function waitConnection (socket, write) {
}
}

function isExternalUrl (url = '') {
return urlPattern.test(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind to check if url.startsWith('http://') || url.startsWith('https://') is faster than regex?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this guaranteed to be "an external url"? Shouldn't the headers be inspected to determine if there is a 3xx code that should be honored?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial thought but status code is not getting passed through the rewriteHeaders method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I would think the solution starts with resolving that issue. I do not see how this simple test is a guarantee of a redirect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If status code is necessary for the checking, I think you can start a PR in fastify-reply-form to achieve it.

};

function proxyWebSockets (source, target) {
function close (code, reason) {
closeWebSocket(source, code, reason)
Expand Down Expand Up @@ -125,7 +129,7 @@ async function httpProxy (fastify, opts) {

function rewriteHeaders (headers) {
const location = headers.location
if (location) {
if (location && !isExternalUrl(location)) {
headers.location = location.replace(rewritePrefix, fastify.prefix)
}
if (oldRewriteHeaders) {
Expand Down
25 changes: 25 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ async function run () {
return 'this is a'
})

origin.get('/redirect', async (request, reply) => {
return reply.redirect(302, 'https://fastify.io')
})

origin.post('/this-has-data', async (request, reply) => {
if (request.body.hello === 'world') {
reply.header('location', '/something')
Expand Down Expand Up @@ -58,6 +62,27 @@ async function run () {
t.equal(resultA.body, 'this is a')
})

test('redirects passthrough', async t => {
const server = Fastify()
server.register(proxy, {
upstream: `http://localhost:${origin.server.address().port}`
})

await server.listen(0)
t.tearDown(server.close.bind(server))

const {
headers: { location },
statusCode
} = await got(
`http://localhost:${server.server.address().port}/redirect`, {
followRedirect: false
}
)
t.equal(location, 'https://fastify.io')
t.equal(statusCode, 302)
})

test('no upstream will throw', async t => {
const server = Fastify()
server.register(proxy)
Expand Down