Skip to content

Commit

Permalink
chore: cleanup useless decodeURIComponent() calls (#1002)
Browse files Browse the repository at this point in the history
* chore: cleanup useless decodeURIComponent() calls

Signed-off-by: Martin d'Allens <[email protected]>

* chore: try to fix CodeQL failure "Polynomial regular expression"

Fix 1:
\d\.?\d* can backtrack catastrophically
\d(\.\d*)? is safer

Fix 2:
Useless parenthesis around "enc:"

Fix 3:
The httpTester regex was misleading. It did not really check for "http".
Simplified to show its true meaning. The behaviour should not have changed.

Signed-off-by: Martin d'Allens <[email protected]>

* chore: try to optimize the regex further, to fix CodeQL failure

Signed-off-by: Martin d'Allens <[email protected]>

* chore: consistency between previous changes, docs, and serve_style.js

Signed-off-by: Martin d'Allens <[email protected]>

---------

Signed-off-by: Martin d'Allens <[email protected]>
  • Loading branch information
Caerbannog authored Oct 13, 2023
1 parent 3781054 commit e8f64e2
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 11 deletions.
4 changes: 2 additions & 2 deletions docs/endpoints.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Static images

* All the static image endpoints additionally support following query parameters:

* ``path`` - ``((fill|stroke|width)\:[^\|]+\|)*((enc:.+)|((-?\d+\.?\d*,-?\d+\.?\d*\|)+(-?\d+\.?\d*,-?\d+\.?\d*)))``
* ``path`` - ``((fill|stroke|width)\:[^\|]+\|)*(enc:.+|-?\d+(\.\d*)?,-?\d+(\.\d*)?(\|-?\d+(\.\d*)?,-?\d+(\.\d*)?)+)``

* comma-separated ``lng,lat``, pipe-separated pairs

Expand All @@ -50,7 +50,7 @@ Static images

* e.g. ``path=stroke:yellow|width:2|fill:green|5.9,45.8|5.9,47.8|10.5,47.8|10.5,45.8|5.9,45.8`` or ``path=stroke:blue|width:1|fill:yellow|enc:_p~iF~ps|U_ulLnnqC_mqNvxq`@``

* can be provided multiple times
* can be provided multiple times

* ``latlng`` - indicates coordinates are in ``lat,lng`` order rather than the usual ``lng,lat``
* ``fill`` - color to use as the fill (e.g. ``red``, ``rgba(255,255,255,0.5)``, ``#0000ff``)
Expand Down
11 changes: 4 additions & 7 deletions src/serve_rendered.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { getFontsPbf, getTileUrls, fixTileJSONCenter } from './utils.js';

const FLOAT_PATTERN = '[+-]?(?:\\d+|\\d+.?\\d+)';
const PATH_PATTERN =
/^((fill|stroke|width)\:[^\|]+\|)*((enc:.+)|((-?\d+\.?\d*,-?\d+\.?\d*\|)+(-?\d+\.?\d*,-?\d+\.?\d*)))/;
const httpTester = /^(http(s)?:)?\/\//;
/^((fill|stroke|width)\:[^\|]+\|)*(enc:.+|-?\d+(\.\d*)?,-?\d+(\.\d*)?(\|-?\d+(\.\d*)?,-?\d+(\.\d*)?)+)/;

Check warning on line 25 in src/serve_rendered.js

View workflow job for this annotation

GitHub Actions / ESLint

src/serve_rendered.js#L25

Unsafe Regular Expression (security/detect-unsafe-regex)
const httpTester = /^\/\//;

const mercator = new SphericalMercator();
const getScale = (scale) => (scale || '@1x').slice(1, 2) | 0;
Expand Down Expand Up @@ -158,10 +158,7 @@ const extractPathsFromQuery = (query, transformer) => {
// Iterate through paths, parse and validate them
for (const providedPath of providedPaths) {
// Logic for pushing coords to path when path includes google polyline
if (
providedPath.includes('enc:') &&
PATH_PATTERN.test(decodeURIComponent(providedPath))
) {
if (providedPath.includes('enc:') && PATH_PATTERN.test(providedPath)) {
// +4 because 'enc:' is 4 characters, everything after 'enc:' is considered to be part of the polyline
const encIndex = providedPath.indexOf('enc:') + 4;
const coords = polyline
Expand Down Expand Up @@ -432,7 +429,7 @@ const drawMarkers = async (ctx, markers, z) => {
* @param {number} z Map zoom level.
*/
const drawPath = (ctx, path, query, pathQuery, z) => {
const splitPaths = decodeURIComponent(pathQuery).split('|');
const splitPaths = pathQuery.split('|');

if (!path || path.length < 2) {
return null;
Expand Down
2 changes: 1 addition & 1 deletion src/serve_style.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { validate } from '@maplibre/maplibre-gl-style-spec';

import { getPublicUrl } from './utils.js';

const httpTester = /^(http(s)?:)?\/\//;
const httpTester = /^\/\//;

const fixUrl = (req, url, publicUrl, opt_nokey) => {
if (!url || typeof url !== 'string' || url.indexOf('local://') !== 0) {
Expand Down
2 changes: 1 addition & 1 deletion test/static.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('Static endpoints', function () {
200,
2,
/image\/png/,
'?path=' + decodeURIComponent('enc:{{biGwvyGoUi@s_A|{@'),
'?path=' + encodeURIComponent('enc:{{biGwvyGoUi@s_A|{@'),
);
});
});
Expand Down

0 comments on commit e8f64e2

Please sign in to comment.