fix: fixed issues with req.url and added more tests

packages/app/src/app.ts

@@ -310,9 +310,9 @@ export class App<Req extends Request = Request, Res extends Response = Response>
 
     const exts = this.applyExtensions || extendMiddleware<RenderOptions>(this)
 
-    req.originalUrl = req.url || req.originalUrl
+    req.originalUrl = req.originalUrl || req.url
 
-    const pathname = getPathname(req.originalUrl)
+    const pathname = getPathname(req.url)
 
     const matched = this.#find(pathname)
 
@@ -338,13 +338,11 @@ export class App<Req extends Request = Request, Res extends Response = Response>
         path: '/'
       })
     }
-
     mw.push({
       handler: this.noMatchHandler,
       type: 'mw',
       path: '/'
     })
-
     const handle = (mw: Middleware) => async (req: Req, res: Res, next?: NextFunction) => {
       const { path, handler, regex } = mw
 
@@ -353,15 +351,14 @@ export class App<Req extends Request = Request, Res extends Response = Response>
       try {
         params = regex ? getURLParams(regex, pathname) : {}
       } catch (e) {
-        console.error(e)
         if (e instanceof URIError) return res.sendStatus(400) // Handle malformed URI
-        else throw e
+        return this.onError(e, req, res)
       }
 
       // Warning: users should not use :wild as a pattern
       let prefix = path
       if (regex) {
-        for (const key of regex.keys) {
+        for (const key of regex.keys as string[]) {
           if (key === 'wild') {
             prefix = prefix.replace('*', params.wild)
           } else {

packages/app/src/request.ts

@@ -16,16 +16,16 @@
 
 const trustRemoteAddress = ({ socket }: Pick<Request, 'headers' | 'socket'>) => {
   const val = socket.remoteAddress
-  if (typeof val === 'string') return compile(val.split(',').map((x) => x.trim()))
-  return compile(val || [])
+  if (typeof val !== 'string') return compile(val || [])
+  return compile(val.split(',').map((x) => x.trim()))
 }
 
 export const getProtocol = (req: Request): Protocol => {
   const proto = `http${req.secure ? 's' : ''}`
 
   if (!trustRemoteAddress(req)) return proto
 
-  const header = (req.headers['X-Forwarded-Proto'] as string) || proto
+  const header = (req.get('X-Forwarded-Proto') as string) || proto
 
   const index = header.indexOf(',')
 
@@ -34,9 +34,7 @@
 
 export const getHostname = (req: Request): string | undefined => {
   let host: string = req.get('X-Forwarded-Host') as string
-
-  if (!host || !trustRemoteAddress(req)) host = req.get('Host') as string
-
+  if (!host) host = req.get('Host') as string
   if (!host) return
 
   // IPv6 literal support
@@ -94,11 +92,11 @@
   acceptsCharsets: (...charsets: string[]) => AcceptsReturns
   acceptsLanguages: (...languages: string[]) => AcceptsReturns
   is: (...types: string[]) => boolean
   cookies?: any
   signedCookies?: any
   secret?: string | string[]
   fresh?: boolean
   stale?: boolean
   body?: any
   app?: App
 }

packages/proxy-addr/src/index.ts

@@ -137,7 +137,7 @@ function parseNetmask(netmask: string) {
  * @param trust
  * @public
  */
-export function proxyaddr(req: Req, trust: Trust): string {
+function proxyaddr(req: Req, trust: Trust): string {
   const addrs = alladdrs(req, trust)
 
   return addrs[addrs.length - 1]
@@ -160,7 +160,6 @@ function trustMulti(subnets: (IPv4 | IPv6)[]) {
       let trusted = ip
       if (kind !== subnetkind) {
         if (subnetkind === 'ipv4' && !(ip as IPv6).isIPv4MappedAddress()) continue
-
         if (!ipconv) ipconv = subnetkind === 'ipv4' ? (ip as IPv6).toIPv4Address() : (ip as IPv4).toIPv4MappedAddress()
 
         trusted = ipconv
@@ -193,5 +192,4 @@ function trustSingle(subnet: IPv4 | IPv6) {
   }
 }
 
-export { alladdrs as all }
-export { compile }
+export { compile, alladdrs as all, proxyaddr }

packages/req/src/fresh.ts

@@ -34,7 +34,6 @@ function isStale(etag: string, noneMatch: string) {
 export function fresh(reqHeaders: IncomingHttpHeaders, resHeaders: OutgoingHttpHeaders) {
   const modifiedSince = reqHeaders['if-modified-since']
   const noneMatch = reqHeaders['if-none-match']
-
   if (!modifiedSince && !noneMatch) return false
 
   const cacheControl = reqHeaders['cache-control']

tests/core/app.test.ts

@@ -1,13 +1,11 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi } from 'vitest'
 import http from 'node:http'
 import { readFile } from 'node:fs/promises'
 import { App } from '../../packages/app/src/index'
 import { renderFile } from 'eta'
-import type { PartialConfig } from 'eta/dist/types/config'
 import { InitAppAndTest } from '../../test_helpers/initAppAndTest'
 import { makeFetch } from 'supertest-fetch'
-import { vi } from 'vitest'
 import { View } from '../../packages/app/src/view'
 
 describe('Testing App', () => {
@@ -53,6 +51,20 @@ describe('Testing App', () => {
 
     await fetch('/').expect(500, 'Ouch, you hurt me on / page.')
   })
+  it('Defaults to onError when `TESTING` env is enabled', async () => {
+    vi.stubEnv('TESTING', '')
+    const app = new App()
+
+    app.use((_req, _res, _next) => {
+      throw new Error('you')
+    })
+
+    const server = app.listen()
+    const fetch = makeFetch(server)
+
+    await fetch('/').expect(500, 'you')
+    vi.unstubAllEnvs()
+  })
 
   it('App works with HTTP 1.1', async () => {
     const app = new App()
@@ -513,6 +525,14 @@ describe('HTTP methods', () => {
 
     await fetch('/hello', { method: 'HEAD' }).expect(404)
   })
+  it('Returns statusCode 204 when no handler is present and the request is `HEAD`', async () => {
+    const app = new App()
+    app.get('/', (_, res, next) => {
+      next()
+    })
+    const fetch = makeFetch(app.listen())
+    await fetch('/', { method: 'HEAD' }).expectStatus(204)
+  })
 })
 
 describe('Route handlers', () => {
@@ -737,18 +757,16 @@ describe('Subapps', () => {
 
     app.route('/path').get((_, res) => res.send('Hello World'))
   })
-  /* it('req.originalUrl does not change', async () => {
+  it('req.originalUrl does not change', async () => {
     const app = new App()
 
     const subApp = new App()
 
-    subApp.get('/route', (req, res) =>
+    subApp.get('/route', (req, res) => {
       res.send({
-        origUrl: req.originalUrl,
-        url: req.url,
-        path: req.path
+        origUrl: req.originalUrl
       })
-    )
+    })
 
     app.use('/subapp', subApp)
 
@@ -757,11 +775,9 @@ describe('Subapps', () => {
     const fetch = makeFetch(server)
 
     await fetch('/subapp/route').expect(200, {
-      origUrl: '/subapp/route',
-      url: '/route',
-      path: '/route'
+      origUrl: '/subapp/route'
     })
-  }) */
+  })
 
   it('lets other wares handle the URL if subapp doesnt have that path', async () => {
     const app = new App()
@@ -909,6 +925,16 @@ describe('Subapps', () => {
     const fetch = makeFetch(server)
     await fetch('/%').expect(400, 'Bad Request')
   })
+  it('should return status of 500 if `getURLParams` has an error', async () => {
+    global.decodeURIComponent = (...args) => {
+      throw new Error('an error was throw here')
+    }
+    const app = new App({
+      onError: (err, req, res) => res.status(500).end(err.message)
+    })
+    app.get('/:id', (_, res) => res.send('hello'))
+    await makeFetch(app.listen())('/123').expect(500, 'an error was throw here')
+  })
   it('handles errors by parent when no onError specified', async () => {
     const app = new App({
       onError: (err, req, res) => res.status(500).end(`Ouch, ${err} hurt me on ${req.path} page.`)
@@ -1002,6 +1028,19 @@ describe('Template engines', () => {
         expect(str).toEqual('Hello from v1rtl')
       })
     })
+    it('should expose app._locals', () => {
+      const app = new App({
+        settings: {
+          views: `${process.cwd()}/tests/fixtures/views`
+        }
+      })
+      app.engine('eta', renderFile)
+
+      app.render('index.eta', {}, { _locals: { name: 'world' } }, (err, str) => {
+        if (err) throw err
+        expect(str).toEqual('Hello from world')
+      })
+    })
     it('should support index files', () => {
       const app = new App({
         settings: {
@@ -1019,13 +1058,16 @@ describe('Template engines', () => {
     })
     describe('errors', () => {
       it('should catch errors', () => {
+        expect.assertions(1)
         const app = new App({
           settings: {
             views: `${process.cwd()}/tests/fixtures`
           }
         })
 
         class TestView {
+          path = 'something'
+          constructor(...args) {}
           render() {
             throw new Error('oops')
           }
@@ -1188,6 +1230,18 @@ describe('Template engines', () => {
       })
     })
   })
+
+  it('uses the default engine to render', () => {
+    const app = new App()
+    app.set('view engine', '.eta')
+    app.engine('eta', renderFile)
+    app.locals.name = 'v1rtl'
+
+    app.render(`${process.cwd()}/tests/fixtures/views/index`, {}, {}, (err, str) => {
+      if (err) throw err
+      expect(str).toEqual('Hello from v1rtl')
+    })
+  })
 })
 
 describe('App settings', () => {