From 816bb91ea36f68b7244be828d651bb34ff626ef8 Mon Sep 17 00:00:00 2001 From: Yan Wang Date: Tue, 31 Dec 2024 08:20:14 +0800 Subject: [PATCH] chore(sagaRouter): upgrade `path-to-regexp` (#5480) --- .changeset/breezy-ways-care.md | 35 +++++ packages/cmf-router/package.json | 2 +- packages/cmf-router/src/sagaRouter.md | 149 ++++++++++++++------- packages/cmf-router/src/sagaRouter.test.js | 65 ++++++++- packages/cmf/__tests__/matchPath.test.js | 43 ++++++ packages/cmf/package.json | 2 +- packages/cmf/src/matchPath.js | 27 ++-- yarn.lock | 10 +- 8 files changed, 262 insertions(+), 71 deletions(-) create mode 100644 .changeset/breezy-ways-care.md diff --git a/.changeset/breezy-ways-care.md b/.changeset/breezy-ways-care.md new file mode 100644 index 00000000000..d26d87d91bc --- /dev/null +++ b/.changeset/breezy-ways-care.md @@ -0,0 +1,35 @@ +--- +'@talend/react-cmf-router': major +'@talend/react-cmf': major +--- + +BREAKING CHANGE: Upgraded path-to-regexp from 3.x to 8.x + +This upgrade was necessary to resolve security vulnerabilities. The new version introduces two breaking changes that require updates to your application: + +1. Optional Path Parameter Syntax Change +- Old syntax: `/resources/:id?` +- New syntax: `/resources{/id}` + +This change is required because in path-to-regexp 8.x, the `?` character is reserved for query parameters and will throw a parsing error when used at the end of a path. + +2. Root Path Matching Behavior Change +- In v3.x, root path `/` would match any path starting with `/` +- In v8.x, root path `/` only matches exactly `/` +- To match both root and child paths, use the wildcard pattern `/{*path}` + +Example migration: +```javascript +// Before +const routes = { + '/': rootSaga, + '/resources/:id?': resourceSaga +}; + +// After +const routes = { + '/{*path}': rootSaga, // if you want to match all routes + '/resources{/id}': resourceSaga +}; +``` +For more details about path matching and troubleshooting, see [path-to-regexp documentation](https://github.com/pillarjs/path-to-regexp#errors). \ No newline at end of file diff --git a/packages/cmf-router/package.json b/packages/cmf-router/package.json index d725b674236..fdbfee877e2 100644 --- a/packages/cmf-router/package.json +++ b/packages/cmf-router/package.json @@ -29,7 +29,7 @@ "connected-react-router": "^6.9.3", "history": "^5.3.0", "lodash": "^4.17.21", - "path-to-regexp": "^3.3.0", + "path-to-regexp": "^8.2.0", "prop-types": "^15.8.1", "react-redux": "^7.2.9", "react-router": "~6.3.0", diff --git a/packages/cmf-router/src/sagaRouter.md b/packages/cmf-router/src/sagaRouter.md index 441dac5a077..ef7b92f5d02 100644 --- a/packages/cmf-router/src/sagaRouter.md +++ b/packages/cmf-router/src/sagaRouter.md @@ -48,10 +48,11 @@ yield spawn(routerSaga, history, routes); ``` ## Matching pattern + ```javascript const routes = { - "/datasets/add": saga1, - "/connections/:datastoreid/edit/add-dataset": saga2 + '/datasets/add': saga1, + '/connections/:datastoreid/edit/add-dataset': saga2, }; ``` @@ -65,9 +66,9 @@ and that we have the following configuration ```javascript const routes = { - "/datasets": datasets, - "/datasets/add": datasetsSaga, - "/connections/add": connectionsSaga + '/datasets': datasets, + '/datasets/add': datasetsSaga, + '/connections/add': connectionsSaga, }; ``` @@ -90,10 +91,11 @@ and that we have the following configuration ```javascript const routes = { - "/datasets/add": datasetsSaga, - "/connections/add": connectionsSaga + '/datasets/add': datasetsSaga, + '/connections/add': connectionsSaga, }; ``` + only `datasetsSaga` will be executed. Now the route is changed to `localhost/connections/add` @@ -116,35 +118,34 @@ so saga of `/datasets` will be restarted when route changes. Optionally, if you want to run a saga only on exact match, you can pass a configuration `runOnExactMatch` as true, then saga will be started when its route exactly match current location, and will be stopped when change to any other route. - ```javascript -import { sagaRouter } from '@talend/react-cmf'; import { browserHistory as history } from 'react-router'; +import { sagaRouter } from '@talend/react-cmf'; + const CANCEL_ACTION = 'CANCEL_ACTION'; // route configuration, a url fragment match with a generator const routes = { - '/datasets': { - // runOnExactMatch: true, - restartOnRouteChange: true, - saga: function* datasets(notUsed, isExact) { - if (!isExact) { - return; - } - yield take(CANCEL_ACTION); - yield put({ - type: REDIRECT_CONNECTION_ADD_DATASET_CANCEL, - cmf: { - routerReplace: `/connections/${datastoreId}/edit`, - }, - }); - }, - }, - '/datasets/add': function* addDataset() {} + '/datasets': { + // runOnExactMatch: true, + restartOnRouteChange: true, + saga: function* datasets(notUsed, isExact) { + if (!isExact) { + return; + } + yield take(CANCEL_ACTION); + yield put({ + type: REDIRECT_CONNECTION_ADD_DATASET_CANCEL, + cmf: { + routerReplace: `/connections/${datastoreId}/edit`, + }, + }); + }, + }, + '/datasets/add': function* addDataset() {}, }; ``` - ### Partial route matching Given the webapp url is `localhost/datasets/add/connection/add` @@ -153,10 +154,11 @@ and that we have the following configuration ```javascript const routes = { - "/datasets/add": datasetsSaga, - "/connections/add": connectionsSaga + '/datasets/add': datasetsSaga, + '/connections/add': connectionsSaga, }; ``` + only `datasetsSaga` will be executed. because the route key can be matched on any part of the url. @@ -171,57 +173,61 @@ and that we have the following configuration ```javascript const routes = { - "/datasets/add": datasetsSaga, - "/datasets/add/connection/add": datasetConnectionsSaga, - "/connection/add": connectionsSaga, + '/datasets/add': datasetsSaga, + '/datasets/add/connection/add': datasetConnectionsSaga, + '/connection/add': connectionsSaga, }; ``` + `datasetsSaga`, `datasetConnectionSaga` and `connectionSaga` are running. ### Route matching and route parameters. + Given the webapp url is `localhost/datasets/50/edit` and that we have the following configuration ```javascript -function* editDatasetSaga ({datasetId}){ - // do something +function* editDatasetSaga({ datasetId }) { + // do something } const routes = { - "/datasets/add": datasetsSaga, - "/datasets/:datasetId/edit": editDatasetSaga + '/datasets/add': datasetsSaga, + '/datasets/:datasetId/edit': editDatasetSaga, }; ``` + only `editDatasetsSaga` will be executed and :datasetId will be resolved and given to the running saga as a parameter. url parameters are resolved and given to the executed saga in form of an object, because we can match on many of them. ```javascript -function* connectionSaga ({connectionId, datasetId}){ - // do something +function* connectionSaga({ connectionId, datasetId }) { + // do something } const routes = { - "/datasets/add": datasetsSaga, - "/datasets/:datasetId/edit": editDatasetSaga, - "/datasets/:datasetId/connections/:connectionId": connectionSaga + '/datasets/add': datasetsSaga, + '/datasets/:datasetId/edit': editDatasetSaga, + '/datasets/:datasetId/connections/:connectionId': connectionSaga, }; ``` ### Route matching with route parameter change + Given the webapp url is `localhost/datasets/50/edit` and that we have the following configuration ```javascript -function* editDatasetSaga ({datasetId}){ - // do something +function* editDatasetSaga({ datasetId }) { + // do something } const routes = { - "/datasets/add": datasetsSaga, - "/datasets/:datasetId/edit": editDatasetSaga + '/datasets/add': datasetsSaga, + '/datasets/:datasetId/edit': editDatasetSaga, }; ``` @@ -234,24 +240,65 @@ the `editDatasetsSaga` is cancelled, and when its done, restarted with the new v Only sagas matching on a route which parameter change are restarted. ### Route matching with optionnal parameters + Given the webapp url is `localhost/datasets/add/550` and that we have the following configuration ```javascript -function* editDatasetSaga ({datasetId}){ - // do something +function* editDatasetSaga({ datasetId }) { + // do something } const routes = { - "/datasets/add/:connectionId?": datasetsSaga, - "/datasets/:datasetId/edit": editDatasetSaga + '/datasets/add{/:connectionId}': datasetsSaga, + '/datasets/:datasetId/edit': editDatasetSaga, }; ``` + datasetSaga will be executed if the route change to `localhost/datasets/add` -the `datasetsSaga` will be restarted since it still match on `/datasets/add/:connectionId?` route and that the parameter has changed from being a value to being absent. +the `datasetsSaga` will be restarted since it still match on `/datasets/add{/:connectionId}` route and that the parameter has changed from being a value to being absent. + +the {/:connectionId} at the end of path means /connectionId is optional. + +### Root Path Matching + +The root path `/` has special matching behavior that's important to understand: + +1. Exact root path matching: + +```javascript +const routes = { + '/': function* rootSaga() { + yield take('SOMETHING'); + }, +}; +``` + +- Only matches exactly `/` +- Does not match child routes like `/tasks` or `/users/123` +- This is because path-to-regexp treats the root path `/` differently than other routes - it won't do partial matching even when `exact` is false +- If you want `/` to match any path that starts with `/`, you need to use a wildcard pattern like `/{*path}` + +2. Matching root and all child routes: + +```javascript +const routes = { + '/{*path}': function* rootSaga({ path }) { + yield take('SOMETHING'); + }, +}; +``` + +- Matches both root path `/` and all child routes +- For root path `/`, params will be empty `{}` +- For child routes, `params.path` will contain the remaining path: + - `/tasks` → `{ path: 'tasks' }` + - `/tasks/123` → `{ path: 'tasks/123' }` + +This pattern is particularly useful when you need a saga to run for all routes in your application while still being able to access the current route path. -the ? at the end of the parameter define that it is optional. +For more details about path matching and troubleshooting, see [path-to-regexp documentation](https://github.com/pillarjs/path-to-regexp#errors). diff --git a/packages/cmf-router/src/sagaRouter.test.js b/packages/cmf-router/src/sagaRouter.test.js index ff22e135f91..b8e012cedf2 100644 --- a/packages/cmf-router/src/sagaRouter.test.js +++ b/packages/cmf-router/src/sagaRouter.test.js @@ -1,5 +1,6 @@ -import { spawn, take, cancel } from 'redux-saga/effects'; import { createMockTask } from '@redux-saga/testing-utils'; +import { cancel, spawn, take } from 'redux-saga/effects'; + import sagaRouter from './sagaRouter'; describe('sagaRouter import', () => { @@ -356,4 +357,66 @@ describe('sagaRouter route and route params', () => { spawn(routes['/matchingroute/:id'], { id: 'anotherId' }, true), ); }); + + it('should handle optional route parameters', () => { + const mockTask = createMockTask(); + function getMockedHistory() { + let count = 0; + return { + get location() { + if (count === 0) { + count = 1; + return { + pathname: '/matchingroute/tasks/taskId-123', + }; + } + return { + pathname: '/matchingroute/tasks', + }; + }, + }; + } + const routes = { + '/matchingroute/:resource{/:optional}': function* matchingSaga() { + yield take('SOMETHING'); + }, + }; + const gen = sagaRouter(getMockedHistory(), routes); + + expect(gen.next().value).toEqual( + spawn( + routes['/matchingroute/:resource{/:optional}'], + { resource: 'tasks', optional: 'taskId-123' }, + true, + ), + ); + expect(gen.next(mockTask).value).toEqual(take('@@router/LOCATION_CHANGE')); + + // optional parameter is removed, saga should be restarted + const expectedCancelYield = cancel(mockTask); + expect(gen.next({ type: '@@router/LOCATION_CHANGE' }).value).toEqual(expectedCancelYield); + expect(gen.next().value).toEqual( + spawn(routes['/matchingroute/:resource{/:optional}'], { resource: 'tasks' }, true), + ); + }); + + it('should start root path saga when on child route', () => { + const mockHistory = { + get location() { + return { + pathname: '/tasks', + }; + }, + }; + const routes = { + '/{*path}': function* rootSaga() { + yield take('SOMETHING'); + }, + }; + const gen = sagaRouter(mockHistory, routes); + + // Root saga should be started with isExact=true for wildcard path + expect(gen.next().value).toEqual(spawn(routes['/{*path}'], { path: 'tasks' }, true)); + expect(gen.next().value).toEqual(take('@@router/LOCATION_CHANGE')); + }); }); diff --git a/packages/cmf/__tests__/matchPath.test.js b/packages/cmf/__tests__/matchPath.test.js index ccb874181f2..6bd56e96c3e 100644 --- a/packages/cmf/__tests__/matchPath.test.js +++ b/packages/cmf/__tests__/matchPath.test.js @@ -7,6 +7,7 @@ describe('matchPath', () => { const pathname = '/'; const match = matchPath(pathname, path); expect(match.url).toBe('/'); + expect(match.isExact).toBe(true); }); }); @@ -16,6 +17,7 @@ describe('matchPath', () => { const pathname = '/somewhere'; const match = matchPath(pathname, path); expect(match.url).toBe('/somewhere'); + expect(match.isExact).toBe(true); }); it('returns correct url at "/somewhere/else"', () => { @@ -23,6 +25,7 @@ describe('matchPath', () => { const pathname = '/somewhere/else'; const match = matchPath(pathname, path); expect(match.url).toBe('/somewhere'); + expect(match.isExact).toBe(false); }); }); @@ -34,6 +37,46 @@ describe('matchPath', () => { const pathname = '/somewhere'; const match = matchPath(pathname, options); expect(match.url).toBe('/somewhere'); + expect(match.isExact).toBe(true); + }); + }); + + describe('with path="/{*path}"', () => { + it('returns correct match at root "/"', () => { + const path = '/{*path}'; + const pathname = '/'; + const match = matchPath(pathname, path); + expect(match.url).toBe('/'); + expect(match.params).toEqual({}); + expect(match.isExact).toBe(true); + }); + + it('returns correct match and params for child route "/tasks"', () => { + const path = '/{*path}'; + const pathname = '/tasks'; + const match = matchPath(pathname, path); + expect(match.url).toBe('/tasks'); + expect(match.params).toEqual({ path: 'tasks' }); + expect(match.isExact).toBe(true); + }); + + it('returns correct match and params for nested route "/tasks/123"', () => { + const path = '/{*path}'; + const pathname = '/tasks/123'; + const match = matchPath(pathname, path); + expect(match.url).toBe('/tasks/123'); + expect(match.params).toEqual({ path: 'tasks/123' }); + expect(match.isExact).toBe(true); + }); + }); + describe('with optinal parameter path="/matchingroute/:resource{/:optional}"', () => { + it('returns correct match and params for child route "/tasks"', () => { + const path = '/matchingroute/:resource{/:optional}'; + const pathname = '/matchingroute/tasks/taskId-123'; + const match = matchPath(pathname, path); + expect(match.url).toBe('/matchingroute/tasks/taskId-123'); + expect(match.params).toEqual({ resource: 'tasks', optional: 'taskId-123' }); + expect(match.isExact).toBe(true); }); }); }); diff --git a/packages/cmf/package.json b/packages/cmf/package.json index 5d430ffddbf..df6dcc020ea 100644 --- a/packages/cmf/package.json +++ b/packages/cmf/package.json @@ -52,7 +52,7 @@ "invariant": "^2.2.4", "lodash": "^4.17.21", "nested-combine-reducers": "^1.2.2", - "path-to-regexp": "^3.3.0", + "path-to-regexp": "^8.2.0", "prop-types": "^15.8.1", "react-immutable-proptypes": "^2.2.0", "react-redux": "^7.2.9", diff --git a/packages/cmf/src/matchPath.js b/packages/cmf/src/matchPath.js index 95c0e734dc9..98d0f0af733 100644 --- a/packages/cmf/src/matchPath.js +++ b/packages/cmf/src/matchPath.js @@ -2,7 +2,7 @@ * Beware! Do not modify. Forked from react-router V4 * Will be available as a dependency */ -import pathToRegexp from 'path-to-regexp'; +import { pathToRegexp } from 'path-to-regexp'; const patternCache = {}; const cacheLimit = 10000; @@ -14,9 +14,8 @@ const compilePath = (pattern, options) => { if (cache[pattern]) return cache[pattern]; - const keys = []; - const re = pathToRegexp(pattern, keys, options); - const compiledPattern = { re, keys }; + const { regexp, keys } = pathToRegexp(pattern, options); + const compiledPattern = { re: regexp, keys }; if (cacheCount < cacheLimit) { cache[pattern] = compiledPattern; @@ -38,18 +37,22 @@ export default function matchPath(pathname, options = {}) { if (!match) return null; - const [url, ...values] = match; + const url = match[0]; const isExact = pathname === url; if (exact && !isExact) return null; + const params = {}; + for (let i = 1; i < match.length; i++) { + if (match[i] === undefined) continue; + const key = keys[i - 1]; + params[key.name] = match[i]; + } + return { - path, // the path pattern used to match - url: path === '/' && url === '' ? '/' : url, // the matched portion of the URL - isExact, // whether or not we matched exactly - params: keys.reduce((memo, key, index) => { - memo[key.name] = values[index]; // eslint-disable-line no-param-reassign - return memo; - }, {}), + path, + url: path === '/' && url === '' ? '/' : url, + isExact, + params, }; } diff --git a/yarn.lock b/yarn.lock index 9f4749361ff..8157e7fe574 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14953,16 +14953,16 @@ path-to-regexp@^2.2.1: resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-2.4.0.tgz#35ce7f333d5616f1c1e1bfe266c3aba2e5b2e704" integrity sha512-G6zHoVqC6GGTQkZwF4lkuEyMbVOjoBKAEybQUypI1WTkqinCOrq2x6U2+phkJ1XsEMTy4LjtwPI7HW+NVrRR2w== -path-to-regexp@^3.3.0: - version "3.3.0" - resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-3.3.0.tgz#f7f31d32e8518c2660862b644414b6d5c63a611b" - integrity sha512-qyCH421YQPS2WFDxDjftfc1ZR5WKQzVzqsp4n9M2kQhVOo/ByahFoUNJfl58kOcEGfQ//7weFTDhm+ss8Ecxgw== - path-to-regexp@^6.2.1, path-to-regexp@^6.3.0: version "6.3.0" resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-6.3.0.tgz#2b6a26a337737a8e1416f9272ed0766b1c0389f4" integrity sha512-Yhpw4T9C6hPpgPeA28us07OJeqZ5EzQTkbfwuhsUg0c237RomFoETJgmp2sa3F/41gfLE6G5cqcYwznmeEeOlQ== +path-to-regexp@^8.2.0: + version "8.2.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-8.2.0.tgz#73990cc29e57a3ff2a0d914095156df5db79e8b4" + integrity sha512-TdrF7fW9Rphjq4RjrW0Kp2AW0Ahwu9sRGTkS6bvDi0SCwZlEZYmcfDbEsTz8RVk0EHIS/Vd1bv3JhG+1xZuAyQ== + path-type@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-4.0.0.tgz#84ed01c0a7ba380afe09d90a8c180dcd9d03043b"