Skip to content

Commit

Permalink
More redirect and rewrite fixes (#885)
Browse files Browse the repository at this point in the history 
* serveRedirect: Fix a condition check

* Add test for 30x redirects

* Improve CRA tests

* Fix file shadowing

* Add tests for shadowing and 404

* Fix 404 redirects behavior

* Add tests for 404 redirect rules

* Update rules-proxy.test.js

* Remove a redudndant test

* Make redirect test more robust

* Formatting

* Remove unused imports

* File ending fixes for Windows

* Use request.get for headers

* Functions: Make sure body is undefined when not provided

* Syntax formatting

* Add tests for echo functions

* Add echo function

* Tests: Add CRA function tests with rewrites

* Test Utils: Improve random port

* Dev: Fallback to random port for functions server
  • Loading branch information
@RaeesBhatti
RaeesBhatti authored May 12, 2020
1 parent 19c2a7b commit 9e3eeb6
Show file tree
Hide file tree
Showing 22 changed files with 346 additions and 82 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
"eslint-plugin-import": "^2.17.3",
"eslint-plugin-node": "^8.0.1",
"eslint-plugin-prettier": "^3.1.0",
"form-data": "^3.0.0",
"from2-string": "^1.1.0",
"gh-release": "^3.5.0",
"globby": "^10.0.1",
Expand Down
13 changes: 8 additions & 5 deletions src/commands/dev/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ function addonUrl(addonUrls, req) {
return addonUrl ? `${addonUrl}${m[2]}` : null
}

async function isStatic(pathname, publicFolder) {
async function getStatic(pathname, publicFolder) {
const alternatives = alternativePathsFor(pathname).map(p => path.resolve(publicFolder, p.substr(1)))

for (const i in alternatives) {
const p = alternatives[i]
try {
const pathStats = await stat(p)
if (pathStats.isFile()) return true
if (pathStats.isFile()) return '/'+path.relative(publicFolder, p)
} catch (err) {
// Ignore
}
Expand Down Expand Up @@ -273,6 +273,8 @@ async function serveRedirect(req, res, proxy, match, options) {
}
}

const staticFile = await getStatic(req.url, options.publicFolder)
if (staticFile) req.url = staticFile
const reqUrl = new url.URL(
req.url,
`${req.protocol || (req.headers.scheme && req.headers.scheme + ':') || 'http:'}//${req.headers['host'] ||
Expand All @@ -283,8 +285,9 @@ async function serveRedirect(req, res, proxy, match, options) {
return render404(options.publicFolder)
}

if (match.force || (!(await isStatic(reqUrl.pathname, options.publicFolder) || options.framework) && match.status !== 404)) {
if (match.force || (!(staticFile && options.framework))) {
const dest = new url.URL(match.to, `${reqUrl.protocol}//${reqUrl.host}`)
const destStaticFile = await getStatic(dest.pathname, options.publicFolder)
if (isRedirect(match)) {
res.writeHead(match.status, {
Location: match.to,
Expand All @@ -309,8 +312,8 @@ async function serveRedirect(req, res, proxy, match, options) {
const destURL = dest.pathname + (urlParams.toString() && '?' + urlParams.toString())

let status
if (match.force || isInternal(destURL) || !options.framework) {
req.url = destURL
if (match.force || isInternal(destURL) || (!staticFile && !options.framework && destStaticFile)) {
req.url = destStaticFile ? destStaticFile : destURL
status = match.status
console.log(`${NETLIFYDEVLOG} Rewrote URL to `, req.url)
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/detect-server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ module.exports.serverSettings = async (devConfig, flags, projectDir, log) => {
settings.port = port

settings.jwtRolePath = devConfig.jwtRolePath || 'app_metadata.authorization.roles'
settings.functionsPort = await getPort({ port: settings.functionsPort || 34567 })
settings.functionsPort = await getPort({ port: settings.functionsPort || 0 })
settings.functions = devConfig.functions || settings.functions

return settings
Expand Down
62 changes: 58 additions & 4 deletions src/utils/rules-proxy.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,38 @@ test('parseFile: netlify.toml', async t => {
status: 200,
},
{
path: '/*',
to: '/index.html',
path: '/foo',
to: '/not-foo',
status: 200,
force: false,
},
{
path: '/foo.html',
to: '/not-foo',
status: 200,
},
{
path: '/not-foo',
to: '/foo',
status: 200,
force: true,
},
{
path: '/test-404a',
to: '/foo',
status: 404,
},
{
path: '/test-404b',
to: '/foo',
status: 404,
},
{
path: '/test-404c',
to: '/foo',
status: 404,
force: true,
},
]

t.deepEqual(rules, expected)
Expand Down Expand Up @@ -58,11 +85,38 @@ test('parseRules', async t => {
status: 200,
},
{
path: '/*',
to: '/index.html',
path: '/foo',
to: '/not-foo',
status: 200,
force: false,
},
{
path: '/foo.html',
to: '/not-foo',
status: 200,
},
{
path: '/not-foo',
to: '/foo',
status: 200,
force: true,
},
{
path: '/test-404a',
to: '/foo',
status: 404,
},
{
path: '/test-404b',
to: '/foo',
status: 404,
},
{
path: '/test-404c',
to: '/foo',
status: 404,
force: true,
},
]

t.deepEqual(rules, expected)
Expand Down
19 changes: 8 additions & 11 deletions src/utils/serve-functions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,27 +121,24 @@ function createHandler(dir) {
}
const { functionPath } = functions[func]

const body = request.body.toString()
var isBase64Encoded = Buffer.from(body, 'base64').toString('base64') === body
const body = request.get('content-length') ? request.body.toString() : undefined
let isBase64Encoded = false
if (body) isBase64Encoded = Buffer.from(body, 'base64').toString('base64') === body

let remoteAddress =
request.headers['x-forwarded-for'] || request.headers['X-Forwarded-for'] || request.connection.remoteAddress || ''
remoteAddress = remoteAddress
.split(remoteAddress.includes('.') ? ':' : ',')
.pop()
.trim()
let remoteAddress = request.get('x-forwarded-for') || request.connection.remoteAddress || ''
remoteAddress = remoteAddress.split(remoteAddress.includes('.') ? ':' : ',').pop().trim()

let requestPath = request.path
if (request.headers['x-netlify-original-pathname']) {
requestPath = request.headers['x-netlify-original-pathname']
if (request.get('x-netlify-original-pathname')) {
requestPath = request.get('x-netlify-original-pathname')
delete request.headers['x-netlify-original-pathname']
}

const event = {
path: requestPath,
httpMethod: request.method,
queryStringParameters: queryString.parse(request.url.split(/\?(.+)/)[1]),
headers: Object.assign({}, request.headers, { 'client-ip': remoteAddress }),
headers: { ...request.headers, 'client-ip': remoteAddress },
body: body,
isBase64Encoded: isBase64Encoded
}
Expand Down
93 changes: 71 additions & 22 deletions tests/cra.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
const fs = require('fs')
const path = require('path')
const util = require('util')
const { spawn } = require('child_process')
const url = require('url')
const test = require('ava')
const fetch = require('node-fetch')
const mkdirp = require('mkdirp')
const cliPath = require('./utils/cliPath')
const { randomPort } = require('./utils/')
const sitePath = path.join(__dirname, 'site-cra')

const fileWrite = util.promisify(fs.writeFile)

let ps, host, port

test.before(async t => {
Expand Down Expand Up @@ -39,13 +35,13 @@ test.before(async t => {
})
})

test('netlify dev cra: homepage', async t => {
test('homepage', async t => {
const response = await fetch(`http://${host}:${port}/`).then(r => r.text())

t.regex(response, /Web site created using create-react-app/)
})

test('netlify dev cra: static/js/bundle.js', async t => {
test('static/js/bundle.js', async t => {
const response = await fetch(`http://${host}:${port}/static/js/bundle.js`)
const body = await response.text()

Expand All @@ -55,37 +51,46 @@ test('netlify dev cra: static/js/bundle.js', async t => {
t.regex(body, /webpackBootstrap/)
})

test('netlify dev cra: static file under build/', async t => {
const publicPath = path.join(sitePath, 'public')
await mkdirp(publicPath)

const expectedContent = '<html><h1>Test content'

await fileWrite(path.join(publicPath, 'test.html'), expectedContent)

test('static file under public/', async t => {
const response = await fetch(`http://${host}:${port}/test.html`)
const body = await response.text()

t.is(response.status, 200)
t.truthy(response.headers.get('content-type').startsWith('text/html'))
t.is(body, expectedContent)
t.is(body, '<html><h1>Test content')
})

test('netlify dev cra: force rewrite', async t => {
const publicPath = path.join(sitePath, 'public')
await mkdirp(publicPath)
test('redirect test', async t => {
const requestURL = new url.URL(`http://${host}:${port}/something`)
const response = await fetch(requestURL, { redirect: 'manual' })

await fileWrite(path.join(publicPath, 'force.html'), '<html><h1>This should never show')
const expectedUrl = new url.URL(requestURL.toString())
expectedUrl.pathname = '/otherthing.html'

const response = await fetch(`http://${host}:${port}/force.html`)
t.is(response.status, 301)
t.is(response.headers.get('location'), expectedUrl.toString())
t.is(await response.text(), 'Redirecting to /otherthing.html')
})

test('normal rewrite', async t => {
const response = await fetch(`http://${host}:${port}/doesnt-exist`)
const body = await response.text()

t.is(response.status, 200)
t.truthy(response.headers.get('content-type').startsWith('text/html'))
t.regex(body, /Web site created using create-react-app/)
})

test('netlify dev cra: robots.txt', async t => {
test('force rewrite', async t => {
const response = await fetch(`http://${host}:${port}/force.html`)
const body = await response.text()

t.is(response.status, 200)
t.truthy(response.headers.get('content-type').startsWith('text/html'))
t.is(body, '<html><h1>Test content')
})

test('robots.txt', async t => {
const response = await fetch(`http://${host}:${port}/robots.txt`)
const body = await response.text()

Expand All @@ -95,6 +100,50 @@ test('netlify dev cra: robots.txt', async t => {
t.regex(body, /# https:\/\/www.robotstxt.org\/robotstxt.html/)
})


test('functions rewrite echo without body', async t => {
const response = await fetch(`http://${host}:${port}/api/echo?ding=dong`).then(r => r.json())

t.is(response.body, undefined)
t.deepEqual(response.headers, {
accept: '*/*',
'accept-encoding': 'gzip,deflate',
'client-ip': '127.0.0.1',
connection: 'close',
host: `${host}:${port}`,
'user-agent': 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)',
'x-forwarded-for': '::ffff:127.0.0.1',
})
t.is(response.httpMethod, 'GET')
t.is(response.isBase64Encoded, false)
t.is(response.path, '/api/echo')
t.deepEqual(response.queryStringParameters, { ding: 'dong' })
})

test('functions rewrite echo with body', async t => {
const response = await fetch(`http://${host}:${port}/api/echo?ding=dong`, {
method: 'POST',
body: 'some=thing',
}).then(r => r.json())

t.is(response.body, 'some=thing')
t.deepEqual(response.headers, {
'accept': '*/*',
'accept-encoding': 'gzip,deflate',
'client-ip': '127.0.0.1',
'connection': 'close',
'host': `${host}:${port}`,
'content-type': 'text/plain;charset=UTF-8',
'content-length': '10',
'user-agent': 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)',
'x-forwarded-for': '::ffff:127.0.0.1',
})
t.is(response.httpMethod, 'POST')
t.is(response.isBase64Encoded, false)
t.is(response.path, '/api/echo')
t.deepEqual(response.queryStringParameters, { ding: 'dong' })
})

test.after.always('cleanup', async t => {
if (ps && ps.pid) ps.kill(process.platform !== 'win32' ? 'SIGHUP' : undefined)
})
Loading

0 comments on commit 9e3eeb6

Please sign in to comment.