Unnecessary url escaping breaks request paths

[whaaaley]

Describe the bug

I wanted to test out netlify functions using netlify-cli. It seems there's an extra / character getting added then escaped.

This project works fine when deployed on netlify and when running without netlify-dev. I'm sure it's a simple fix I need to make in my config but I can't seem to find any information in docs or any threads online.

Configuration

[build]
  command = 'make'
  publish = 'public'

[dev]
  command = 'make start'
  framework = '#custom'
  publish = 'public'
  targetPort = 3000
  port = 3001

[functions]
  directory = 'functions'

[[headers]]
  for = '/*'
  [headers.values]
    cache-control = 'public,max-age=86400,immutable'

[[headers]]
  for = '/cache/*'
  [headers.values]
    cache-control = 'public,max-age=604800,immutable'

[[headers]]
  for = '/fonts/*'
  [headers.values]
    cache-control = 'public,max-age=31536000,immutable'

[[redirects]]
  from = '/*'
  to = '/index.html'
  status = 200

CLI Output

[whaaaley]

Removing the redirect fixed this issue for me during dev, however, not having this will 100% break my site in production. Perhaps I need to adjust my own server to handle the forwarding that netlify-dev does?

[[redirects]]
  from = '/*'
  to = '/index.html'
  status = 200

Edit: I looked into this a bit more. It seems that my server is just getting requested exactly what is shown in my screenshot above. There's not much I can do about this on my end that isn't super janky. Removing the slashes from the redirects sections doesn't do anything either.

Edit 2: Might be the same as #2708 or #1242

[knoxjeffrey]

Just to add that I have seen the double slash with my Middleman server. I did multiple installs of netlify-cli to see at which version this started happening and it was on this release. With version 3.37.21 a request to the homepage on my local server logs out as

Request: /index.html

and on version 3.37.22 it logs out as

Request: /%2Findex.html

which causes it to error and not find the page.

[whaaaley]

Hello. Just an update. This is still happening with 6.1.0. If I'm not mistaken, this seems to be more of an issue with escaping characters in the URL to be "safe" rather than an extra slash being prepended to the request. I think I should have named this issue "Unnecessary url escaping breaks request paths".

Perhaps a flag could disable this feature for the time being? It's quite a nuisance to use netlify dev for me and my team because of this. We have to comment out the redirect during dev but make sure it's available for production.

For the time being, I'll have to write a flag to run our dev server in "Netlify Dev Mode" to handle the escaped proxied request paths.

Edit:

  if (args['--netlify'] === true) {
    req.url = decodeURIComponent(req.url)
    req.url = req.url.replace(/^\/\//g, '/')
  }

[erezrokah]

Hi all 👋 And sorry for the delayed response on this.

Can someone post a reproduction scenario/public repo with this issue?

I tried with a sample middleman project, but couldn't reproduce

[whaaaley]

@erezrokah Hello 👋 ... I don't have a public project I can share atm however I can reproduce this easily in vanilla node with http.createServer just by logging req.url.

server.js

const http = require('http')

const server = http.createServer(function (req, res) {
  console.log('>>>', req.url)
})

server.listen(3000, function () {
  console.log('listening on port 3000')
})

netlify.toml

[dev]
  command = 'node server.js'
  framework = '#custom'
  publish = 'public'
  targetPort = 3000
  port = 3001

Start project

netlify dev

Finally... Go to browser and paste localhost:3001/main.css
In terminal you'll see >>> /%2Fmain.css

Edit:
I ran this test on version netlify-cli@6.7.5

Edit2:
It appears that the escaping will only happen if it's a commonly known file type extension like .css. Request paths with no file type extension or uncommon extensions are fine.

Also, while doing this test, I ran into another weird issue where "uncommon" file extensions would get a /index.html or .htm extension appended to the end. This is definitely not a desired behavior.

Expected behavior would either redirect completely to /index.html if specified in the redirects or return a 404. There appears to be a lot of filtering happening that really messes with my dev server. I would much prefer a completely pure proxied request rather than whatever filtering is going on here. It's causing a lot of headaches for my team and I.

[TheDarkTurtle]

I ran into a similar issue, it actually breaks if the force option is set to true:

[[redirects]]
    from = "/*"
    to = "https://example.com/:splat"
    status = 200
    force = true

Running netlify dev will lead to:

TypeError: next is not a function
    at HttpProxyMiddleware.<anonymous> (.../node_modules/netlify-cli/node_modules/http-proxy-middleware/dist/http-proxy-middleware.js:38:17)
    at Generator.next (<anonymous>)
    at .../node_modules/netlify-cli/node_modules/http-proxy-middleware/dist/http-proxy-middleware.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (.../node_modules/netlify-cli/node_modules/http-proxy-middleware/dist/http-proxy-middleware.js:4:12)
    at HttpProxyMiddleware.middleware (.../node_modules/netlify-cli/node_modules/http-proxy-middleware/dist/http-proxy-middleware.js:26:47)
    at proxyToExternalUrl (.../node_modules/netlify-cli/src/utils/proxy.js:69:10)
    at serveRedirect (.../node_modules/netlify-cli/src/utils/proxy.js:232:14)

The encoding mismatch between the proxy config and the request will cause the shouldProxy of the http-proxy-middleware to be false and the next function to be called, which here is not a function.

Most likely due to this change (the issue is not reproducible with v3.37.21), hope it helps!

[coelmay]

I have long had the issue with a single project using Astro where I would see the error

[astro] 500 /%2Findex.html 

Reproducing the error in a basic Astro build has proved rather elusive. However, I have found some further clues.

In the netlify.toml are

[build]
  command = "yarn dev"
  publish = "dist"

## other bits and bobs

[[redirects]]
  from = "/*"
  to = "/404"
  status = 404

With these settings, if there is a dist directory in the project root, the above 500 error is produced for any page. If either the [[redirects]] for 404 or the publish = "dist" is removed, everything works fine. If there is no dist directory in the project root, the error is not produced.

The strange thing is for the project this has always occurred on, the 404 page is never served when the 500 error is produced. However with the barebones test project I have worked on today, the 404 page is served and the 500 error is not produced (as demonstrated below.)

Screen-Recording.mov

[whaaaley]

Yeah it's pretty clear that netlify-cli is escaping URLs when it should not. It should just proxy the request as-is when using an existing server. Or at least provide pure req path passthrough as a flag. Though tbh it shouldn't even work like this at all. It doesn't do this in production.

If you're able to, you can fix this issue temporarily by undoing the double encoding like this before the request gets passed into your server. I don't know how Astro works, perhaps a middleware or a wrapper?

// https://github.com/netlify/cli/issues/2710
req.url = decodeURIComponent(req.url)
req.url = req.url.replace(/^\/\//g, '/')

This fixes the issue for slashes only. If you encounter any other characters that get double escaped your paths will break again. I don't have a comprehensive list of characters that netlify-cli escapes.

[whaaaley]

@coelmay Your issue with the 404 however is working as expected. You're redirecting from /* which is everything to the 404 page which is /404. If you want to design a custom 404 page this is not where you should be denoting that.

The error in this thread about double escaping slashes only happens when you're using a third party server and the redirects at the same time. I believe the effect you're seeing is because the 404 page redirect may not get proxied. Or if it is then I'm even more confused. What a mess.

[coelmay]

The error I am having (as I referenced, and as I mentioned I was not able to fully replicate in the demonstration of which I also mentioned and of which I added a short clip of) is of receiving in error whereby Astro is asked to load the path

/%2Findex.html

which of course does not exist. This is the same as mentioned here. While the framework/method used is possibly (likely) different, the outcome is the same.

This rule only redirects paths that do not exist to the 404 page.

[[redirects]]
  from = "/*"
  to = "/404"
  status = 404

If force = true was added, every path would show the 404 page. This is not the issue. This is standard. I'm not using a node server.js. I'm using Astro. Go check it out.

[coelmay]

Hi all 👋 And sorry for the delayed response on this.

Can someone post a reproduction scenario/public repo with this issue?

I tried with a sample middleman project, but couldn't reproduce

I only suspect this is related as I am seeing the same behaviour as @knoxjeffrey with the /%2Findex.html when using Astro. Basic reproduction (though slightly different to my other project as mentioned in a previous post) with notes/results of testing in coelmay/astro-ntl-bug @erezrokah

If this is indeed a separate/unrelated, happy to open a new issue.

[coelmay]

I have just updated to netlify-cli/9.13.0 and the I mentioned/demonstrated previously is now non-existent @erezrokah

Edit: Retracting previous statement as untrue.