-
-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: Upgrade MapLibre to v6 to address CVE #8035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
22516ee
10c9890
ebfb9d1
7fc48b5
d521195
88c466c
642a61b
3458286
0cf4a5a
a7ab40e
535ea6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ node_modules | |
|
|
||
| build/* | ||
| !build/plotcss.js | ||
| !build/maplibre_worker.js | ||
| !build/README.md | ||
|
|
||
| dist/*.LICENSE.txt | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -220,8 +220,10 @@ files. | |||||||||||||||||||
|
|
||||||||||||||||||||
| #### Other npm scripts that may be of interest in development | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - `npm run preprocess`: pre-processes the css and svg source file in js. This | ||||||||||||||||||||
| script must be run manually when updating the css and svg source files. | ||||||||||||||||||||
| - `npm run preprocess`: regenerates the sources that the build depends on, and | ||||||||||||||||||||
| must be run manually when their inputs change. Commit the result. | ||||||||||||||||||||
| `build/README.md` describes the committed files under `build/` and what | ||||||||||||||||||||
| triggers each one. | ||||||||||||||||||||
|
Comment on lines
+223
to
+226
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| ## Testing | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -386,7 +388,14 @@ If you would like a link to the dev build for your PR but don't have permission | |||||||||||||||||||
| - All tasks can be run using [`npm run-script`](https://docs.npmjs.com/cli/run-script) | ||||||||||||||||||||
| - Tests are in `test/`; they are partitioned into `image` and `jasmine` tests | ||||||||||||||||||||
| - Test dashboard and image viewer code is in `devtools/` | ||||||||||||||||||||
| - Built files are in `build/` (the files in here are git-ignored, except for `plotcss.js`) | ||||||||||||||||||||
| - Built files are in `build/` (mostly git-ignored; see `build/README.md`) | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| ### The bundled maplibre-gl worker | ||||||||||||||||||||
|
|
||||||||||||||||||||
| `map` traces render through `maplibre-gl`, which parses tiles in a web worker. | ||||||||||||||||||||
| As of v6, the library no longer includes a UMD bundle with this code. As such, | ||||||||||||||||||||
| it's now necessary to extract it and include it as part of the build process. | ||||||||||||||||||||
| See `tasks/util/bundle_maplibre_worker.js` for more information. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ## Trace module design | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1 +1,15 @@ | ||||||
| Directory of non-distributed built files | ||||||
| ## Directory of generated build files | ||||||
|
|
||||||
| Most of what lands here is scratch output that git and npm both ignore. The dev server and the image tests read their bundles from here. | ||||||
|
|
||||||
| There are exceptions for the files listed below. These files are used in the build process, but need to be generated prior to that. | ||||||
|
|
||||||
| - `plotcss.js`: The stylesheet compiled from `src/css/style.scss`. `src/core.js` requires it, so it must resolve. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| - `maplibre_worker.js`: The maplibre-gl web worker, bundled into one standalone script and exported as a string. `src/plots/map/map.js` requires it and turns the string into a blob URL. | ||||||
|
|
||||||
| These files are committed and shipped in the npm package. Anyone who installs plotly.js and bundles it from `lib/` or `src/` resolves all of these, so leaving any out of the package breaks that build. These are specifically allowlisted in `.gitignore` and `.npmignore` for that reason. | ||||||
|
|
||||||
| `npm run preprocess` regenerates these files. Run that command and commit the result whenever the input changes. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| - For `maplibre_worker.js` this means every time `maplibre-gl` gets upgraded. A stale worker ships against a newer core, so CI runs `npm run maplibre-worker-diff-check` to catch that. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| - For `plotcss.js` this means every time a stylesheet under `src/css/` changes, including the partials that `style.scss` pulls in with `@use`. The same command also writes `dist/plotly.css`, which strict-CSP applications load instead of the inlined styles. | ||||||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| - Update `maplibre-gl` to v6 to address [CVE-2026-85061](https://github.com/advisories/GHSA-jrc7-96c5-q579) [[#8035](https://github.com/plotly/plotly.js/pull/8035)] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a |
||
| - `maplibre-gl` v6 dropped WebGL1 support, so some older browsers won't be able to use the map traces. Safari 15, Chrome 56, Firefox 51 and later are now required for the map traces. | ||
| - Box and lasso selection of `scattermap` points are now supported on a rotated or pitched map | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would put this as a separate bullet point (with a link to the same PR) since it's a related but entirely separate change |
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,21 @@ | ||
| 'use strict'; | ||
|
|
||
| var maplibregl = require('maplibre-gl'); | ||
| // maplibre-gl v6 is ESM only, so request the bundle by its explicit path. | ||
| const maplibregl = require('maplibre-gl/dist/maplibre-gl.mjs'); | ||
|
|
||
| // Get the maplibre-gl worker src to load it as v5 used to. | ||
| // This (mostly) follows the guidance from the MapLibre install | ||
| // guide: https://maplibre.org/maplibre-gl-js/docs/#installation | ||
| const maplibreWorkerSource = require('../../../build/maplibre_worker'); | ||
| let workerUrlIsSet = false; | ||
|
|
||
| // Point maplibre-gl at the worker | ||
| const setWorkerUrl = () => { | ||
| // Only load the inlined blob when the first map is requested to avoid taking a chunk of memory | ||
| if (workerUrlIsSet) return; | ||
| workerUrlIsSet = true; | ||
| maplibregl.setWorkerUrl(URL.createObjectURL(new Blob([maplibreWorkerSource], { type: 'text/javascript' }))); | ||
| }; | ||
|
|
||
| var Lib = require('../../lib'); | ||
| var geoUtils = require('../../lib/geo_location_utils'); | ||
|
|
@@ -111,23 +126,30 @@ proto.createMap = function (calcData, fullLayout, resolve, reject) { | |
| mapOptions.fitBoundsOptions = { padding: constants.fitBoundsPadding }; | ||
| } | ||
|
|
||
| setWorkerUrl(); | ||
|
|
||
| // Create the map! | ||
| const map = (self.map = new maplibregl.Map(mapOptions).addControl( | ||
| new maplibregl.AttributionControl({ compact: true }) | ||
| )); | ||
|
|
||
| var requestedIcons = {}; | ||
| map.on('styleimagemissing', function (e) { | ||
| var id = e.id; | ||
| const requestedIcons = {}; | ||
| map.setMissingStyleImageResolver((id) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. Will this get rid of the console warnings that show up when using Maki icons? |
||
| if (!requestedIcons[id] && /^[a-zA-Z0-9-]+$/.test(id)) { | ||
| requestedIcons[id] = true; | ||
| var img = new Image(15, 15); | ||
| img.onload = function () { | ||
| map.addImage(id, img, { sdf: true }); | ||
| }; | ||
| img.crossOrigin = 'Anonymous'; | ||
| img.src = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@8.2.0/icons/' + id + '.svg'; | ||
| // Use a promise so that maplibre-gl awaits the resolution before treating the image as missing | ||
| requestedIcons[id] = new Promise((resolve) => { | ||
| const img = new Image(15, 15); | ||
| img.onload = () => { | ||
| map.addImage(id, img, { sdf: true }); | ||
| resolve(); | ||
| }; | ||
| img.onerror = () => resolve(); | ||
| img.crossOrigin = 'Anonymous'; | ||
| img.src = `https://cdn.jsdelivr.net/npm/@mapbox/maki@8.2.0/icons/${id}.svg`; | ||
| }); | ||
| } | ||
|
|
||
| return requestedIcons[id]; | ||
| }); | ||
|
|
||
| map.setTransformRequest(function (url) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@camdecoster Since this step has nothing to do with generated types, could you move it to a separate job?