SSR: potential security risk

[doejon]

When using SSR, the current path is stored globally at the node_module level which will lead to path information being shared across synchronous requests:

https://github.com/Paratron/hookrouter/blob/53d65f8c90a6812abcbfa9ef55e1b87d57ea9e6f/src/router.js#L109

It's easy to reproduce with a simple timeout in an express app:

const route = async (req: Request, res: Response): Promise<void> => {
    setPath(req.originalUrl);
    let x = (): void => {};
    const p = new Promise(resolve => {
        x = resolve;
    });
    setTimeout(x, 3000);
    await p;
    const app = renderToString(<App />)
    res.send(html(app));
};

Let's assume the timeout represents unresolved requests being processed before we render the app. The app itself will look something like this:

const routes = {
    '/route1': (): ReactElement => <Route1 />,
    '/route2': (): ReactElement => <Route2 />,
};

const App = (props: { session: Session }): React.ReactElement => { return useRoutes(routes);

When you now visit localhost:/route1 and localhost:/route2 before route1 starts rendering, you'll get route2's content shipped within localhost:/route1.

Since the website will not only be called by the same user in two different tabs, but by different users sending all kinds of parameters within their URLs, the previously set path might expose certain information you definitely do not want to see in another user's server-side rendered page.

Potential fix(es):

• create a new httprouter instance on a per-request/render basis;
• use some sort of context/hook setup;

[Paratron]

This is only possible, because you do your setPath() at the very beginning, then you wait for some unknown time and then you eventually render something. If you move the setPath() to the line immediately above renderToString(), the problem goes away. But only if the renderToString() does not render asynchronously as well.

I am currently thinking about rewriting parts of hookrouter in favor of routing contexts (to allow parallel renders) but this leads to the problem that navigate() isnt simply callable from any point anymore. I need to tell navigate explicitly which routing context it needs to navigate and this reduces the ease of use of the library significantly...