RTFDB-4842: Improve user input handling in "mapbox-gl-geocoder"

CHANGELOG.md

@@ -1,12 +1,18 @@
 ## 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;"`)
+- 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
 

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);
@@ -794,10 +812,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 });
@@ -836,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();
@@ -933,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);
@@ -1087,10 +1110,16 @@ MapboxGeocoder.prototype = {
     this._renderMessage(errorMessage);
   },
 
+  _renderInputTooLongError: function() {
+    var errorMessage = "<div class='mapbox-gl-geocoder--error'>Your search is too long. Please limit it to " + MAX_INPUT_LENGTH + " characters.</div>"
+    this._renderMessage(errorMessage);
+  },
+
   _renderMessage: function(msg){
     this._typeahead.update([]);
     this._typeahead.selected = null;
     this._typeahead.clear();
+    this._hideAttribution();
     this._typeahead.renderError(msg);
   },
 

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,

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,');

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"');
 })