refactor(middleware): enhance subdomain handling and URL rewriting logic#66
refactor(middleware): enhance subdomain handling and URL rewriting logic#66Stivenjs wants to merge 2 commits into
Conversation
This commit updates the middleware to detect subdomains in both production and development environments. It implements URL rewriting to redirect users to the appropriate store page based on the detected subdomain, improving navigation and user experience. Additionally, the matcher configuration has been simplified to exclude specific paths, streamlining route handling.
WalkthroughA new React client component was added to handle dynamic store routes, displaying the store name from the URL. The middleware logic was updated to dynamically detect subdomains, rewrite requests based on subdomain and environment, and expand its scope with a broader regex matcher, excluding certain static and API routes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Middleware
participant StorePage
User->>Middleware: Request to root or path with subdomain (e.g., tienda.fasttify.com)
Middleware->>Middleware: Detect subdomain from hostname
alt Path is root "/"
Middleware->>Middleware: Rewrite path to /[subdomain]
else Path does not start with subdomain
Middleware->>Middleware: Rewrite path to /[subdomain]/[original path]
end
Middleware->>StorePage: Forward rewritten request
StorePage->>User: Render store page with store name from URL
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
middleware.ts (1)
13-14: Extract environment configuration to a constant.The environment check is inline, but it would be cleaner and more testable to extract this to a configuration object.
app/[store]/page.tsx (2)
1-1: Consider using a server component for better performance.This component doesn't require client-side interactivity and could benefit from server-side rendering.
-'use client' - -import { useParams } from 'next/navigation' +interface StorePageProps { + params: { + store: string + } +} -export default function StorePage() { - const params = useParams() - const store = params.store +export default function StorePage({ params }: StorePageProps) { + const { store } = params
11-11: Avoid hard-coding text for internationalization support.The text "Tienda:" is hard-coded in Spanish. Consider using a translation system for better internationalization support.
- <h1>Tienda: {store}</h1> + <h1>Store: {store}</h1>Alternatively, implement a proper i18n solution for multi-language support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/[store]/page.tsx(1 hunks)middleware.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Aplicar Prettier
- GitHub Check: Ejecutar pruebas unitarias
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
middleware.ts (1)
82-82:Details
❓ Verification inconclusive
Consider the security implications of the broad matcher pattern.
The new matcher pattern excludes only specific paths but applies middleware to all other routes. Ensure this doesn't inadvertently expose routes that should not have subdomain handling applied.
🏁 Script executed:
#!/bin/bash # Description: Find all route files to verify they're compatible with subdomain handling # Find all page.tsx and route.ts files fd -e tsx -e ts '(page|route)\.(tsx?|js)$' app/ # Check for any API routes that might be affected fd -e ts -e js 'route\.(ts|js)$' app/Length of output: 1403
Re-evaluate matcher scope for subdomain middleware
Current pattern in
middleware.ts:matcher: ['/((?!api|_next/static|_next/image|favicon.ico).*)'],This excludes only
/api,/_next/static,/_next/image, and/favicon.ico, so it will run on every other route under theappdirectory. Confirm that this broad coverage is intended—pages like/login,/first-steps, your dynamic[slug]dashboards, etc., will all invoke the subdomain middleware.
- Verify there are no legacy
pages/apiroutes (or other endpoint handlers) that need to remain outside this middleware.- Check public/static asset endpoints (e.g.,
/robots.txt,/sitemap.xml) and add them to the exclusion list if they shouldn’t be intercepted.- If only store-specific or subdomain-bound routes should trigger the middleware, narrow the regex to target those paths explicitly.
|
|
||
| export async function middleware(request: NextRequest) { | ||
| const path = request.nextUrl.pathname | ||
| const hostname = request.headers.get('host') || '' |
There was a problem hiding this comment.
Validate the hostname to prevent header spoofing.
The hostname is retrieved directly from request headers without validation, which could be manipulated by clients. This poses a security risk as attackers could potentially bypass subdomain-based access controls.
Apply this diff to add hostname validation:
- const hostname = request.headers.get('host') || ''
+ const hostname = request.headers.get('host') || ''
+
+ // Validate hostname to prevent header spoofing
+ const allowedDomains = isProduction
+ ? ['fasttify.com']
+ : ['localhost']
+
+ const isValidHostname = allowedDomains.some(domain =>
+ hostname === domain || hostname.endsWith(`.${domain}`)
+ )
+
+ if (!isValidHostname) {
+ return NextResponse.redirect(new URL('/', request.url))
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hostname = request.headers.get('host') || '' | |
| const hostname = request.headers.get('host') || '' | |
| // Validate hostname to prevent header spoofing | |
| const allowedDomains = isProduction | |
| ? ['fasttify.com'] | |
| : ['localhost'] | |
| const isValidHostname = allowedDomains.some(domain => | |
| hostname === domain || hostname.endsWith(`.${domain}`) | |
| ) | |
| if (!isValidHostname) { | |
| return NextResponse.redirect(new URL('/', request.url)) | |
| } |
🤖 Prompt for AI Agents
In middleware.ts at line 11, the hostname is obtained directly from request
headers without validation, which risks header spoofing. To fix this, validate
the hostname against a whitelist of allowed hostnames or use a robust parsing
method to ensure it matches expected patterns before using it for any access
control decisions. Reject or handle requests with invalid or unexpected
hostnames appropriately to prevent security bypass.
| // Si hay un subdominio y estamos en la raíz, reescribir a la ruta de la tienda | ||
| if (subdomain && subdomain !== 'www' && path === '/') { | ||
| // Reescribir la URL para mostrar la página de la tienda | ||
| const url = request.nextUrl.clone() | ||
| url.pathname = `/${subdomain}` | ||
| return NextResponse.rewrite(url) | ||
| } | ||
|
|
||
| // Si hay un subdominio y la ruta no empieza con el subdominio, agregar el prefijo | ||
| if (subdomain && subdomain !== 'www' && !path.startsWith(`/${subdomain}`)) { | ||
| const url = request.nextUrl.clone() | ||
| url.pathname = `/${subdomain}${path}` | ||
| return NextResponse.rewrite(url) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate URL rewriting logic to eliminate duplication.
Both conditional blocks check for subdomain existence and perform similar URL rewriting. This can be simplified into a single block.
Apply this diff to consolidate the logic:
- // Si hay un subdominio y estamos en la raíz, reescribir a la ruta de la tienda
- if (subdomain && subdomain !== 'www' && path === '/') {
- // Reescribir la URL para mostrar la página de la tienda
- const url = request.nextUrl.clone()
- url.pathname = `/${subdomain}`
- return NextResponse.rewrite(url)
- }
-
- // Si hay un subdominio y la ruta no empieza con el subdominio, agregar el prefijo
- if (subdomain && subdomain !== 'www' && !path.startsWith(`/${subdomain}`)) {
- const url = request.nextUrl.clone()
- url.pathname = `/${subdomain}${path}`
- return NextResponse.rewrite(url)
- }
+ // Reescribir URLs basadas en subdominios
+ if (subdomain && subdomain !== 'www') {
+ const url = request.nextUrl.clone()
+
+ if (path === '/') {
+ // Si estamos en la raíz, reescribir a la ruta de la tienda
+ url.pathname = `/${subdomain}`
+ } else if (!path.startsWith(`/${subdomain}`)) {
+ // Si la ruta no empieza con el subdominio, agregar el prefijo
+ url.pathname = `/${subdomain}${path}`
+ } else {
+ // La ruta ya tiene el prefijo correcto
+ return NextResponse.next()
+ }
+
+ return NextResponse.rewrite(url)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Si hay un subdominio y estamos en la raíz, reescribir a la ruta de la tienda | |
| if (subdomain && subdomain !== 'www' && path === '/') { | |
| // Reescribir la URL para mostrar la página de la tienda | |
| const url = request.nextUrl.clone() | |
| url.pathname = `/${subdomain}` | |
| return NextResponse.rewrite(url) | |
| } | |
| // Si hay un subdominio y la ruta no empieza con el subdominio, agregar el prefijo | |
| if (subdomain && subdomain !== 'www' && !path.startsWith(`/${subdomain}`)) { | |
| const url = request.nextUrl.clone() | |
| url.pathname = `/${subdomain}${path}` | |
| return NextResponse.rewrite(url) | |
| } | |
| // Reescribir URLs basadas en subdominios | |
| if (subdomain && subdomain !== 'www') { | |
| const url = request.nextUrl.clone() | |
| if (path === '/') { | |
| // Si estamos en la raíz, reescribir a la ruta de la tienda | |
| url.pathname = `/${subdomain}` | |
| } else if (!path.startsWith(`/${subdomain}`)) { | |
| // Si la ruta no empieza con el subdominio, agregar el prefijo | |
| url.pathname = `/${subdomain}${path}` | |
| } else { | |
| // La ruta ya tiene el prefijo correcto | |
| return NextResponse.next() | |
| } | |
| return NextResponse.rewrite(url) | |
| } |
🤖 Prompt for AI Agents
In middleware.ts around lines 31 to 44, the URL rewriting logic for subdomains
is duplicated in two separate if blocks. Consolidate these by combining the
conditions that check for subdomain existence and rewrite the URL accordingly in
a single block. Use a single cloned URL object and adjust the pathname based on
whether the current path is '/' or does not start with the subdomain prefix,
then return the rewritten response once.
| // Detectar subdominios | ||
| let subdomain = '' | ||
| if (isProduction) { | ||
| // En producción: verificar si hay un subdominio (ej: tienda.fasttify.com) | ||
| const parts = hostname.split('.') | ||
| if (parts.length > 2 && hostname.endsWith('fasttify.com')) { | ||
| subdomain = parts[0] | ||
| } | ||
| } else { | ||
| // En desarrollo: usar el formato subdominio.localhost:3000 o localhost:3000 | ||
| if (hostname.includes('.localhost')) { | ||
| subdomain = hostname.split('.')[0] | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor subdomain extraction logic for better maintainability.
The subdomain extraction logic has different branches for production and development, which could be simplified and made more robust.
Apply this diff to improve the subdomain extraction:
- // Detectar subdominios
- let subdomain = ''
- if (isProduction) {
- // En producción: verificar si hay un subdominio (ej: tienda.fasttify.com)
- const parts = hostname.split('.')
- if (parts.length > 2 && hostname.endsWith('fasttify.com')) {
- subdomain = parts[0]
- }
- } else {
- // En desarrollo: usar el formato subdominio.localhost:3000 o localhost:3000
- if (hostname.includes('.localhost')) {
- subdomain = hostname.split('.')[0]
- }
- }
+ // Detectar subdominios
+ const extractSubdomain = (hostname: string, isProduction: boolean): string => {
+ const cleanHostname = hostname.split(':')[0] // Remove port if present
+ const parts = cleanHostname.split('.')
+
+ if (isProduction) {
+ // En producción: verificar si hay un subdominio (ej: tienda.fasttify.com)
+ if (parts.length > 2 && cleanHostname.endsWith('fasttify.com')) {
+ return parts[0]
+ }
+ } else {
+ // En desarrollo: usar el formato subdominio.localhost:3000
+ if (parts.length > 1 && cleanHostname.endsWith('localhost')) {
+ return parts[0]
+ }
+ }
+
+ return ''
+ }
+
+ const subdomain = extractSubdomain(hostname, isProduction)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Detectar subdominios | |
| let subdomain = '' | |
| if (isProduction) { | |
| // En producción: verificar si hay un subdominio (ej: tienda.fasttify.com) | |
| const parts = hostname.split('.') | |
| if (parts.length > 2 && hostname.endsWith('fasttify.com')) { | |
| subdomain = parts[0] | |
| } | |
| } else { | |
| // En desarrollo: usar el formato subdominio.localhost:3000 o localhost:3000 | |
| if (hostname.includes('.localhost')) { | |
| subdomain = hostname.split('.')[0] | |
| } | |
| } | |
| // Detectar subdominios | |
| const extractSubdomain = (hostname: string, isProduction: boolean): string => { | |
| const cleanHostname = hostname.split(':')[0] // Remove port if present | |
| const parts = cleanHostname.split('.') | |
| if (isProduction) { | |
| // En producción: verificar si hay un subdominio (ej: tienda.fasttify.com) | |
| if (parts.length > 2 && cleanHostname.endsWith('fasttify.com')) { | |
| return parts[0] | |
| } | |
| } else { | |
| // En desarrollo: usar el formato subdominio.localhost:3000 | |
| if (parts.length > 1 && cleanHostname.endsWith('localhost')) { | |
| return parts[0] | |
| } | |
| } | |
| return '' | |
| } | |
| const subdomain = extractSubdomain(hostname, isProduction) |
🤖 Prompt for AI Agents
In middleware.ts around lines 16 to 29, the subdomain extraction logic is split
between production and development environments, making it less maintainable.
Refactor this by consolidating the logic into a single, clear function or block
that handles both cases more robustly, ensuring it correctly extracts the
subdomain from hostnames like "subdomain.fasttify.com" in production and
"subdomain.localhost" in development, while simplifying the conditions and
improving readability.
| export default function StorePage() { | ||
| const params = useParams() | ||
| const store = params.store | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>Tienda: {store}</h1> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety and error handling for the store parameter.
The component lacks type checking and doesn't handle cases where the store parameter might be missing or invalid.
Apply this diff to add proper type safety and error handling:
export default function StorePage() {
const params = useParams()
- const store = params.store
+ const store = params.store as string | undefined
+
+ if (!store) {
+ return (
+ <div>
+ <h1>Error: Store not found</h1>
+ </div>
+ )
+ }
return (
<div>
<h1>Tienda: {store}</h1>
</div>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function StorePage() { | |
| const params = useParams() | |
| const store = params.store | |
| return ( | |
| <div> | |
| <h1>Tienda: {store}</h1> | |
| </div> | |
| ) | |
| } | |
| export default function StorePage() { | |
| const params = useParams() | |
| const store = params.store as string | undefined | |
| if (!store) { | |
| return ( | |
| <div> | |
| <h1>Error: Store not found</h1> | |
| </div> | |
| ) | |
| } | |
| return ( | |
| <div> | |
| <h1>Tienda: {store}</h1> | |
| </div> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In app/[store]/page.tsx around lines 5 to 14, the StorePage component lacks type
safety for the params and does not handle cases where the store parameter is
missing or invalid. Add explicit typing for the params object, check if the
store parameter exists and is valid before rendering, and provide fallback UI or
error handling if the store parameter is absent or invalid to ensure robustness.
2 participants
This commit updates the middleware to detect subdomains in both production and development environments. It implements URL rewriting to redirect users to the appropriate store page based on the detected subdomain, improving navigation and user experience. Additionally, the matcher configuration has been simplified to exclude specific paths, streamlining route handling.
Summary by CodeRabbit
New Features
Improvements