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

Redirect urls with multiple slashes in the middle of url with status code 301 #2926

Closed
talaxasy opened this issue Dec 6, 2024 · 5 comments
Closed

Comments

@talaxasy
Copy link

talaxasy commented Dec 6, 2024

Environment

Nuxt config:

export default defineNuxtConfig({
  compatibilityDate: '2024-04-03',
  devtools: { enabled: false }
})

Nuxt project info:

  • Operating System: Linux
  • Node Version: v18.20.3
  • Nuxt Version: 3.14.1592
  • CLI Version: 3.16.0
  • Nitro Version: 2.10.4
  • Package Manager: npm@10.2.3
  • Builder: -
  • User Config: default
  • Runtime Modules: -
  • Build Modules: -

Reproduction

Pure Nitro (works): https://stackblitz.com/edit/github-pznd9a?file=server%2Froutes%2F%5B...%5D.ts
Nuxt app (not works): https://stackblitz.com/edit/nuxt-starter-zzxsrh?file=server%2Fmiddleware%2Fredirects.ts

Describe the bug

I'm trying to create middleware that will redirect urls with multiple slashes in the middle of url with status code 301.

I created such middleware (src/middleware/01.trailing-slashes.global.ts):

export default defineNuxtRouteMiddleware((to) => {
  const event = useRequestEvent();
  let { path, query, hash } = to;
  const normalizedPath = path.replace(//{2,}/g, '/').replace(//+$/, '') || '/';

  if (normalizedPath !== path) {
    const nextRoute = { path: normalizedPath, query, hash };
    setResponseStatus(event, 301);
    return navigateTo(nextRoute, { redirectCode: 301 });
  }
})

But to.path already comes normalized, without slashes.
For example I make req on "http://localhost:3000/produkte////atemwege/prod1" and in middleware get a "/produkte/atemwege/prod1" as you can see I already get to.path without extra slashes.

I even tried with server middleware (src/server/middleware/redirects.ts):

export default defineEventHandler((event) => {
  const pathname = getRequestURL(event).pathname;
  if (pathname.startsWith('/api')) return;
  console.log('Original URL:', event.node.req.url);
  console.log('New request: ' + event.node.req.originalUrl)
})

But event.node.req.ur or event.node.req.originalUrl comes without slashes.

What I want to achieve is I just need to make a redirect for such cases:
http://localhost:3000/a/////b/c (301) -> http://localhost:3000/a/b/c (200)
http://localhost:3000///a///b///c (301) -> http://localhost:3000/a/b/c (200)

What is the current behavior:
http://localhost:3000/a/////b/c (200) -> http://localhost:3000/a/b/c (200)
http://localhost:3000///a///b///c (200) -> http://localhost:3000/a/b/c (200)

Additional context

No response

Logs

No response

@talaxasy
Copy link
Author

talaxasy commented Dec 6, 2024

On "Nuxt app (not works): https://stackblitz.com/edit/nuxt-starter-zzxsrh?file=server%2Fmiddleware%2Fredirects.ts" try to put into the address bar a//b//c, and in the server console you will see modified url like a/b/c not a//b//c

@OskarLebuda
Copy link

OskarLebuda commented Dec 7, 2024

tl;dr

The issue is in the httpxy library, specifically in the urlJoin method: https://github.com/unjs/httpxy/blob/main/src/_utils.ts#L195

.replace(/\/+/g, "/")

Question for @pi0: What should we do about this? Personally, I think this regexp is either unnecessary or needs rewriting. This only happens in the DEV version, because of nitro_dev preset. Production build works fine without any changes.

Debugging and Pinpointing the Issue

I started by looking into what connects Nitro and Nuxt - clearly, it's h3, the heart of the HTTP server. So, I set up some simple code for H3:

import { createApp, defineEventHandler } from 'h3';

export const app = createApp();

app.use(defineEventHandler((event) => {
  console.log('Original URL:', event.node.req.url);
  console.log('New request: ' + event.node.req.originalUrl);
}));

Firing up the server and... no bug: 😭

$ pnpm dev:
...
Original URL: /a//b//c
New request: /a//b//c

That made me realize the problem must be somewhere in Nitro.

So, I looked into how Nitro launches its dev-server. It took a lot of time to go through every line, understand all dependencies and their connections with external packages. I figured out that for the dev-server, Nitro runs the nitro-dev preset. This preset utilizes node:worker_threads and the httpxy library to manage communication between the Worker and the HTTP server created in server.ts:

function createProxy(defaults: ProxyServerOptions = {}) {

Requests are passed down to the worker through the httpxy library. So then, it was time to dig into that... After some searching and debugging, I finally nailed it! 🔥 💪🏻

The whole error is caused by the urlJoin function. Specifically, this piece of code:

.replace(/\/+/g, "/")

This regex processes // into / across the entire string, which means /a//b//c becomes /a/b/c/, and that's exactly how the request hits nuxt-dev as an incoming request.

Ok, it's time to final test:

// middleware/test.ts
export default defineEventHandler((event) => {
  console.log('[Nitro] Middleware handler - url:', event.node.req.originalUrl);
});
With replace Without replace
image image

After removing that replace - everything works perfectly!

@talaxasy
Copy link
Author

talaxasy commented Dec 8, 2024

@OskarLebuda Great researching! Then it would be logical to give the developer the ability to disable or enable URL normalization

@OskarLebuda
Copy link

OskarLebuda commented Dec 9, 2024

@talaxasy this issue is only on dev env. Prod env works as expected 😊

@pi0
Copy link
Member

pi0 commented Jan 7, 2025

Let's track in #630

@pi0 pi0 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2025
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

3 participants