Skip to content

Commit

Permalink
fix(security): soft navigation to not rely on jquery.history (#671)
Browse files Browse the repository at this point in the history
Fixes https://github.com/openwhyd/openwhyd/security/code-scanning/15 and https://github.com/openwhyd/openwhyd/security/code-scanning/14

Contributes to #669.

## Challenge

To implement Openwhyd's soft/ajax-based navigation logic (that does not interrupt the music playback when navigating), `jquery.history` has been used indirectly by a jQuery-based abstraction layer of [`window.history`](https://developer.mozilla.org/en-US/docs/Web/API/Window/history): [jQuery History](https://github.com/yeikos/jquery.history). ([source code](https://github.com/yeikos/jquery.history/blob/master/jquery.history.js))

=> This PR rewrites that logic, using the browser's History API directly.

## Changes

* fix(security): fix: remove `jquery.history`

* update snapshots for approval tests

* `noDefaultJs` is never set => simplify list of included js files

* fix(makefile): add `test-e2e-dev` to open Cypress runner

* make e2e tests pass

* clean-up logs

* use `window.history` explicitly instead of `History`

* call `pushState()` just from `goToPage()`

* fix: `rootUrl` to make it work from any domain

* remove default value for parameter `url`

* fix(sonar): use substring instead of substr (deprecated)

* fix(sonar): remove commented code
  • Loading branch information
adrienjoly committed Aug 31, 2023
1 parent 61babb7 commit 23afd39
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 140 deletions.
5 changes: 5 additions & 0 deletions Makefile
Expand Up @@ -60,6 +60,11 @@ test-e2e: node_modules public/js/bookmarklet.js ## Run tests against a local dat
CYPRESS_SKIP_APPLITOOLS_TESTS=true npm run test:cypress
docker compose stop

test-e2e-dev: node_modules public/js/bookmarklet.js ## Open Cypress test runner against a local database + Openwhyd server
docker compose up --detach --build mongo web
CYPRESS_SKIP_APPLITOOLS_TESTS=true npm run test:cypress:dev
docker compose stop

test-approval: node_modules public/js/bookmarklet.js ## Run approval tests against a local db
docker compose stop
docker compose up --detach mongo
Expand Down
52 changes: 19 additions & 33 deletions app/templates/mainTemplate.js
Expand Up @@ -214,14 +214,8 @@ exports.renderWhydFrame = function (html, params) {
);

out.push(
// ' <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.1/jquery.min.js"></script>',
// ' <script type="text/javascript" src="'+render.urlPrefix+'/js/jquery-1.10.2.min.js"></script>',
' <script src="/js/jquery-1.10.2.min.js"></script>',
' <script src="/js/jquery-migrate-1.2.1.js"></script>',
' <script type="text/javascript" src="' +
render.urlPrefix +
'/js/jquery.history.js"></script>',
// ' <script src="/js/soundmanager2.js"></script>',
' <script src="/js/soundmanager2-nodebug-jsmin.js"></script>',
' <script>soundManager.setup({url: "/swf/", flashVersion: 9, onready: function() {soundManager.isReady=true;}});</script>',
);
Expand Down Expand Up @@ -353,33 +347,25 @@ exports.renderWhydPage = function (params = {}) {
? ''
: ' – Discover and collect the best music tracks from the web');

params.js = (
params.noDefaultJs
? []
: [
'jquery.avgrund.js',
'jquery.tipsy.js', // replaces tooltip.js
'quickSearch.js',
// "md5.js",

'jquery.iframe-post-form.min.js',
'jquery.placeholder.min.js',
'underscore-min.js', // for jquery.mentionsInput.js
'jquery.elastic.js', // for jquery.mentionsInput.js
'jquery.mentionsInput.js',
'ui.js',
'whyd.js', // topicBrowser.js
]
)
.concat(['playem-' + playemFile + '.js'])
.concat([
'playem-youtube-iframe-patch.js',
'whydPlayer.js',
'dndUpload.js',
'WhydImgUpload.js',
])
.concat(params.js || [])
.concat(params.noDefaultJs ? [] : ['facebook.js']);
params.js = [
'jquery.avgrund.js',
'jquery.tipsy.js', // replaces tooltip.js
'quickSearch.js',
// "md5.js",
'jquery.iframe-post-form.min.js',
'jquery.placeholder.min.js',
'underscore-min.js', // for jquery.mentionsInput.js
'jquery.elastic.js', // for jquery.mentionsInput.js
'jquery.mentionsInput.js',
'ui.js',
'whyd.js', // topicBrowser.js
'playem-' + playemFile + '.js',
'playem-youtube-iframe-patch.js',
'whydPlayer.js',
'dndUpload.js',
'WhydImgUpload.js',
'facebook.js',
].concat(params.js || []);

params.css = [
'browse.css',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -30,8 +30,8 @@
"test:unit": "npx --yes mocha test/unit/*.js --exit",
"test:unit:coverage": "npm run coverage:clear && npx --yes nyc --silent $(node -p 'require(`./package.json`).scripts[`test:unit`]') $@",
"test:approval:routes": "ava test/approval/routes/routes.approval.tests.js $@",
"test:approval:routes:update": "npm run test:approval:routes -- --update-snapshots",
"test:approval:routes:start": "START_WITH_ENV_FILE='./env-vars-testing.conf' npm run test:approval:routes -- $@",
"test:approval:routes:start:update": "npm run test:approval:routes:start -- --update-snapshots",
"test:approval:hot-tracks:raw": "npx --yes jest --runInBand test/approval/hot-tracks/hot-tracks.approval.test.js $@",
"test:approval:hot-tracks": "PORT=8080 npm run test:approval:hot-tracks:raw -- $@",
"test:approval:hot-tracks:start": "START_WITH_ENV_FILE='./env-vars-testing.conf' npm run test:approval:hot-tracks:raw -- $@",
Expand Down
1 change: 0 additions & 1 deletion public/js/jquery.history.js

This file was deleted.

0 comments on commit 23afd39

Please sign in to comment.