From 65b120f336bcbb01b1703e11742c4c3799bfa255 Mon Sep 17 00:00:00 2001 From: pahuta Date: Sun, 2 Aug 2026 14:07:45 +0200 Subject: [PATCH 1/2] RTFDB-4842: Improve user input handling in "mapbox-gl-geocoder" - Remove numeric-range bound on coordinate detection regex so it matches how the Geocoding v5 API itself detects coordinate-shaped queries, fixing forward/reverse misclassification (422 errors) for out-of-range coordinate-like input - Require a comma (with optional surrounding whitespace) as the coordinate separator so whitespace-only separated numbers are still treated as a forward query - Deduplicate the coordinate regex in the forward-request branch, reusing the shared utils.REVERSE_GEOCODE_COORD_RGX - Add corresponding test cases and changelog entry https://mapbox.atlassian.net/browse/RTFDB-4842 --- CHANGELOG.md | 4 ++++ lib/index.js | 9 +++++---- lib/utils.js | 9 +++++++-- test/utils.test.js | 6 +++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 728fc6e1..0bc86014 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## HEAD +### Breaking changes ⚠️ + +- Coordinate detection (`options.reverseGeocode`) no longer bounds the numeric range of the input. Any input that looks like coordinates (two numbers separated by a comma, with optional whitespace around the comma, e.g. `"12.45, 12345456"`) is now always treated as a reverse geocode request, regardless of whether the numbers fall within valid latitude/longitude bounds. Previously, inputs with more than 3 integer digits (e.g. `12345456`) were misclassified as a forward geocoding request, which the Geocoding v5 API would still interpret as a reverse request server-side, resulting in a response with an error and status code 422. This change aligns the package's detection logic with how the API itself detects coordinate-shaped queries — the API does not treat whitespace-separated numbers without a comma (e.g. `"12.55 34.87"`) as coordinates, so that input is still handled as a forward geocoding request. + ### Features / Improvements 🚀 - Add `inputTransforms.trimCoordinatesPunctuation` option (defaults to `false`). When enabled, leading/trailing punctuation (e.g. `;`) is trimmed from search input that looks like coordinates (e.g. `"48.774989, 9.155557;"`) diff --git a/lib/index.js b/lib/index.js index 506bbbec..c9d256cb 100644 --- a/lib/index.js +++ b/lib/index.js @@ -794,10 +794,11 @@ MapboxGeocoder.prototype = { }); } break; case GEOCODE_REQUEST_TYPE.FORWARD: { - // Ensure that any reverse geocoding looking request is cleaned up - // to be processed as only a forward geocoding request by the server. - const reverseGeocodeCoordRgx = /^(-?\d{1,3}(\.\d{0,256})?)[, ]+(-?\d{1,3}(\.\d{0,256})?)?$/; - if (reverseGeocodeCoordRgx.test(search)) { + // options.reverseGeocode may be false even though the input looks like + // coordinates. The Geocoding v5 API detects coordinate-shaped queries on + // its own and would otherwise treat this forward request as reverse, so + // replace the comma to keep it unambiguously a forward text query. + if (utils.REVERSE_GEOCODE_COORD_RGX.test(search)) { search = search.replace(/,/g, ' '); } config = extend(config, { query: search }); diff --git a/lib/utils.js b/lib/utils.js index 2356503b..ca1b0845 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -60,11 +60,16 @@ function getAddressInfo(feature) { return addrInfo; } -const REVERSE_GEOCODE_COORD_RGX = /^(-?\d{1,3}(\.\d{0,256})?)[, ]+(-?\d{1,3}(\.\d{0,256})?)$/; +// Matches any two numbers separated by a comma (optionally surrounded by +// whitespace), with no bound on the numeric range. This mirrors how the +// Geocoding v5 API itself detects coordinate-like queries, so a string +// classified here as "coordinates" is always handled as a reverse geocode +// request, never split between forward/reverse. +const REVERSE_GEOCODE_COORD_RGX = /^(-?\d+(\.\d{0,256})?)\s*,\s*(-?\d+(\.\d{0,256})?)$/; // Unanchored version of REVERSE_GEOCODE_COORD_RGX: checks that the string contains // coordinates somewhere in it, regardless of surrounding punctuation/whitespace. -const RELAXED_COORD_RGX = /(-?\d{1,3}(\.\d{0,256})?)[, ]+(-?\d{1,3}(\.\d{0,256})?)/; +const RELAXED_COORD_RGX = /(-?\d+(\.\d{0,256})?)\s*,\s*(-?\d+(\.\d{0,256})?)/; module.exports = { transformFeatureToGeolocationText: transformFeatureToGeolocationText, diff --git a/test/utils.test.js b/test/utils.test.js index ec486c07..e573ecc5 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -9,6 +9,10 @@ test('REVERSE_GEOCODE_COORD_RGX', function (t) { t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('12., 34.'), 'Reverse: "12., 34."'); t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('122, 41'), 'Reverse: "122, 41"'); t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('12, 123'), 'Reverse: "12, 123"'); - t.notOk(utils.REVERSE_GEOCODE_COORD_RGX.test('1234, 4568'), 'Forward: "1234, 4568"'); + t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('1234, 4568'), 'Reverse: "1234, 4568" (no numeric range check, matches API behavior)'); + t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('12.45, 12345456'), 'Reverse: "12.45, 12345456" (out-of-range values are still coordinate-shaped)'); + t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('12.55,34.87'), 'Reverse: "12.55,34.87" (no space around comma)'); + t.ok(utils.REVERSE_GEOCODE_COORD_RGX.test('12.55 , 34.87'), 'Reverse: "12.55 , 34.87" (space before and after comma)'); + t.notOk(utils.REVERSE_GEOCODE_COORD_RGX.test('12.55 34.87'), 'Forward: "12.55 34.87" (no comma, only whitespace-separated)'); t.notOk(utils.REVERSE_GEOCODE_COORD_RGX.test('123 Main'), 'Forward: "123 Main"'); }) \ No newline at end of file From 29f669b33423f1e56fc118a49f765c8b7ce51bf4 Mon Sep 17 00:00:00 2001 From: pahuta Date: Mon, 3 Aug 2026 13:35:02 +0200 Subject: [PATCH 2/2] RTFDB-4842: Improve user input handling in "mapbox-gl-geocoder" Add input length validation and fix suggestions list race conditions https://mapbox.atlassian.net/browse/RTFDB-4842 --- CHANGELOG.md | 2 ++ lib/index.js | 52 +++++++++++++++++++++++++++++++++---------- test/test.geocoder.js | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bc86014..57722196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,12 @@ ### Features / Improvements 🚀 - Add `inputTransforms.trimCoordinatesPunctuation` option (defaults to `false`). When enabled, leading/trailing punctuation (e.g. `;`) is trimmed from search input that looks like coordinates (e.g. `"48.774989, 9.155557;"`) +- Reject search input longer than 256 characters (matching the Geocoding v5 API's own limit) with a dedicated "search is too long" error message, instead of sending it to the API ### Bug fixes 🐛 - Fix reverse geocoding errors caused by leading/trailing whitespace in coordinate input (e.g. `"48.774989, 9.155557 "`) +- Fix an error message not being shown when pasting an invalid value directly (e.g. via keyboard shortcut), instead of typing it character by character ## 5.1.2 diff --git a/lib/index.js b/lib/index.js index c9d256cb..c7d65c3c 100644 --- a/lib/index.js +++ b/lib/index.js @@ -22,6 +22,11 @@ const GEOCODE_REQUEST_TYPE = { const PUNCTUATION_CHARS = new Set([';']); +// Mirrors the Geocoding v5 API's own limit on `search_text` length +// (https://docs.mapbox.com/api/search/geocoding-v5/), and keeps user input +// short enough that regex-based input transforms can't be a ReDoS vector. +const MAX_INPUT_LENGTH = 256; + /** * Don't include this as part of the options object when creating a new MapboxGeocoder instance. */ @@ -317,7 +322,24 @@ MapboxGeocoder.prototype = { const handleKeyDownTypeahead = this._typeahead.handleKeyDown.bind(this._typeahead); const handleKeyUpTypeahead = this._typeahead.handleKeyUp.bind(this._typeahead); - this._typeahead.handleKeyUp + // Suggestions' own `handleKeyUp` is called two different ways: (1) with no + // argument, internally by Suggestions#update() to redraw the list after we + // hand it new data - that must keep working; (2) with a raw keyCode, by + // Suggestions' own native `keyup` listener on the input, which redraws from + // `this.data` regardless of what we're currently showing. That second path + // races with our own rendering (most visibly right after a paste keyboard + // shortcut, where the keyup for the released keys fires after our own paste + // handling and wipes out a just-rendered error with an empty list), and is + // otherwise pure redundant noise since this project always drives the list + // itself via _typeahead.update()/_renderMessage(). So ignore case (2). + this._typeahead.handleKeyUp = function(e) { + if (arguments.length === 0) { + return handleKeyUpTypeahead(); + } + if (this.options.useBrowserFocus && e && e.keyCode === 16) { + e.preventDefault(); + } + }.bind(this); if (this.options.useBrowserFocus) { this._typeahead.handleKeyDown = function(e) { @@ -345,15 +367,6 @@ MapboxGeocoder.prototype = { } handleKeyDownTypeahead(e); }.bind(this); - - this._typeahead.handleKeyUp = function(e) { - if (e && e.keyCode === 16) { - e.preventDefault(); - return; - } - - handleKeyUpTypeahead(e); - } } // Add support for footer. @@ -524,7 +537,6 @@ MapboxGeocoder.prototype = { this._hideLoadingIcon(); this._showGeolocateButton(); - this._hideAttribution(); }.bind(this)); }, @@ -564,6 +576,12 @@ MapboxGeocoder.prototype = { }, _onPaste: function(e){ + // Suggestions binds its own `paste` listener to the same input and, on paste, + // redraws the list from its own (here always-empty) `this.data` - which races + // with and can wipe out whatever we render below. Since this project always + // drives the list itself, stop that listener from running at all. + e.stopImmediatePropagation(); + var value = (e.clipboardData || window.clipboardData).getData('text'); if (value.length >= this.options.minLength) { this._geocode(value); @@ -837,6 +855,11 @@ MapboxGeocoder.prototype = { }, _geocode: function(searchInput) { + if (searchInput.length > MAX_INPUT_LENGTH) { + this._renderInputTooLongError(); + return Promise.resolve(); + } + searchInput = this._transformInput(searchInput); this.inputString = searchInput; this._showLoadingIcon(); @@ -934,7 +957,6 @@ MapboxGeocoder.prototype = { this._typeahead.update(res.features); } else { this._hideClearButton(); - this._hideAttribution(); this._typeahead.selected = null; this._renderNoResults(); this._eventEmitter.emit('results', res); @@ -1088,10 +1110,16 @@ MapboxGeocoder.prototype = { this._renderMessage(errorMessage); }, + _renderInputTooLongError: function() { + var errorMessage = "
Your search is too long. Please limit it to " + MAX_INPUT_LENGTH + " characters.
" + this._renderMessage(errorMessage); + }, + _renderMessage: function(msg){ this._typeahead.update([]); this._typeahead.selected = null; this._typeahead.clear(); + this._hideAttribution(); this._typeahead.renderError(msg); }, diff --git a/test/test.geocoder.js b/test/test.geocoder.js index 79e1dc35..f2414aa4 100644 --- a/test/test.geocoder.js +++ b/test/test.geocoder.js @@ -1485,6 +1485,49 @@ test('geocoder', function(tt) { ); }); + tt.test('geocoder#_renderInputTooLongError', function(t){ + setup({}); + var renderMessageSpy = sinon.spy(geocoder, '_renderMessage'); + + geocoder._renderInputTooLongError(); + t.ok(renderMessageSpy.calledOnce, 'the input too long render method calls the renderMessage method exactly once'); + var calledWithArgs = renderMessageSpy.args[0][0]; + t.ok(calledWithArgs.indexOf('mapbox-gl-geocoder--error') > -1, 'the error message specifies the correct class'); + t.end(); + }); + + tt.test('geocoder#_geocode with input over 256 characters', function(t){ + setup({}); + var renderInputTooLongErrorSpy = sinon.spy(geocoder, '_renderInputTooLongError'); + var forwardGeocodeSpy = sinon.spy(geocoder.geocoderService, 'forwardGeocode'); + var reverseGeocodeSpy = sinon.spy(geocoder.geocoderService, 'reverseGeocode'); + + var tooLongInput = 'a'.repeat(257); + geocoder.query(tooLongInput); + + t.ok(renderInputTooLongErrorSpy.calledOnce, 'the input too long error is rendered'); + t.ok(forwardGeocodeSpy.notCalled, 'no forward geocoding request is made'); + t.ok(reverseGeocodeSpy.notCalled, 'no reverse geocoding request is made'); + t.end(); + }); + + tt.test('geocoder#_geocode with input at 256 characters', function(t){ + t.plan(2); + setup({}); + var renderInputTooLongErrorSpy = sinon.spy(geocoder, '_renderInputTooLongError'); + + var maxLengthInput = 'high' + ' a'.repeat(126); + geocoder.query(maxLengthInput); + geocoder.on( + 'results', + once(function() { + t.ok(renderInputTooLongErrorSpy.notCalled, 'the input too long error is not rendered'); + t.equals(maxLengthInput.length, 256, 'the test input is exactly at the length limit'); + t.end(); + }) + ); + }); + tt.test('error is shown after an error occurred', function(t){ setup({}); geocoder.query('12,');