fix: restore req/res prototypes when a mounted app hands control back - #7428
fix: restore req/res prototypes when a mounted app hands control back#7428lazerg wants to merge 3 commits into
Conversation
|
Great work on identifying the core prototype swap in The approach of capturing To make the fix completely airtight for maintainers and prevent future regressions, there are a few important edge cases that should also be covered in the test suite:
I've put together a PR in #7434 with these comprehensive test cases if you'd like to incorporate them or collaborate to help get this merged quickly! |
|
Added a test for the next(err) path (e418c6f): the parent's error middleware also gets the restored prototypes. The other two cases don't need separate assertions — req.app is defined on the same request object the prototype check already compares, and any custom methods on app.request/app.response come along with that same identity check. |
app.handleswaps thereq/resprototypes for its own, but onlyapp.use()puts them back, inside themounted_appclosure it wraps around a sub-app. Mount an app withrouter.use()instead and the swap is never undone, so every middleware that runs after the sub-app callsnext()readsreq.app,req.ip,req.secureandreq.hostnamethrough the sub-app's settings rather than the parent's.app.handlenow saves the prototypes it replaces and restores them before invoking the callback it was handed, so the cleanup happens on whichever path the app was mounted through. The issue suggested patchingrestore()insiderouter, but the swap originates here, and doing it here also covers any other caller that passes a callback without needing arouterrelease.This covers the prototype half of #7427 only. A router-mounted app still gets no
mountevent and so does not inherittrust proxy, which would needrouterto know about express apps.Fixes #7427