Skip to content
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

Using /*splat in app.all() does not work on v5 #6111

Open
djw-sl opened this issue Oct 31, 2024 · 4 comments
Open

Using /*splat in app.all() does not work on v5 #6111

djw-sl opened this issue Oct 31, 2024 · 4 comments

Comments

@djw-sl
Copy link

djw-sl commented Oct 31, 2024

Hi,

I recently tried to upgrade to v5 from v4 and used the following guide:

https://expressjs.com/en/guide/migrating-5.html#path-syntax

Here is my app.all() that sets headers on all routes:

app.all('/api/*', function (req, res, next) {
  res.setHeader('Access-Control-Allow-Origin', '*');
  res.setHeader(
    'Access-Control-Allow-Headers',
    'Origin, Accepts, Accept, Accept-Version, Content-Length, Content-MD5, Content-Type, Date, X-Api-Version, X-Response-Time, X-CSRF-Token, Authorization',
  );
  res.setHeader('Access-Control-Allow-Methods', '*');
  res.setHeader('Access-Control-Expose-Headers', 'X-Api-Version, X-Request-Id, X-Response-Time');
  res.setHeader('Access-Control-Max-Age', '1000');
  res.setHeader('Cache-Control', 'no-cache');
  res.setHeader('Expires', '-1');
  res.setHeader('Pragma', 'no-cache');
  next();
});

As per documentation:

The wildcard * must have a name, matching the behavior of parameters :, use / *splat instead of / *

I changed it to:

  app.all('/api/*splat'

This does not work. Anything I am doing wrong?

@djw-sl djw-sl added the bug label Oct 31, 2024
@krzysdz
Copy link
Contributor

krzysdz commented Oct 31, 2024

It definitely should work. What exactly doesn't work in your case? Is the handler skipped or do you get an error?

This simplified code works as expected:

const express = require("express");

const app = express();

app.all("/api/*sth", (req, res, next) => {
        res.setHeader("my-header", "a value");
        next()
});

app.get("/*p", (req, res) => res.send("ok"));

const server = app.listen(1234, () =>
        fetch("http://localhost:1234/api/random/path").then(r => {
                console.log(r.headers);
                server.close();
        })
);

Screenshot_2024-10-31-12-09-11-847_com.termux-edit.jpg

@krzysdz
Copy link
Contributor

krzysdz commented Oct 31, 2024

In this case it may be easier to use app.use:

app.use("/api", (req, res, next) => {
    // set headers
    next();
});

.use matches on prefix (if path is specified), so there is no need to use the * parameter.

@Tate-CC
Copy link

Tate-CC commented Nov 29, 2024

Hi @krzysdz 👋

I think I'm also encountering a similar problem. If I have a route on /*splat it fails to match on requests to / whereas using /* would've matched on express 4.

So for example, the file

const express = require("express");

const app = express();

app.get("/*", (req, res) => res.send("ok"));

const server = app.listen(1234, () =>
	fetch("http://localhost:1234/").then(async r => {
					console.log(await r.text());
					server.close();
	})
);

Logs ok on [email protected].

After migrating to [email protected] the file looks like:

const express = require("express");

const app = express();

app.get("/*splat", (req, res) => res.send("ok"));

const server = app.listen(1234, () =>
	fetch("http://localhost:1234/").then(async r => {
					console.log(await r.text());
					server.close();
	})
);

Which logs Cannot GET / (alongside the rest of the HTML 😅).

@krzysdz
Copy link
Contributor

krzysdz commented Nov 29, 2024

Good catch @Tate-CC !

I did not notice it, but the path-to-regexp says

Wildcard parameters match one or more characters across multiple segments.

Looks like an optional group {} must be used - "/{*paramName}" is now (5.x) required for full compatibility with 4.x's "/*".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants