Request URL length <= 2000 checking does not escape '

[Voileexperiments]

Tested on Leaflet 1.7.1, Esri Leaflet 2.5.1.

I've encountered a issue where a request to a Feature layer through resource proxy would fail with 404 error when filter condition reaches specific length. Looking at #983, I found that Request.js is calculating the length of the request to be less than the actual length, and it just so happens hit the 2000 (probably 2048?) character limit so it's incorrectly using GET request, which caused the fail.

https://github.com/Esri/esri-leaflet/blob/master/src/Request.js#L121

With a debugger I see that paramString did not escape ' to %27 while the actual request URL would, hence under-estimating the URL length.

[Voileexperiments]

Example fiddle: https://jsfiddle.net/t9onha6p/

The request URL sent out is

https://sampleserver6.arcgisonline.com/arcgis/rest/services/Earthquakes_Since1970/MapServer/0/query?returnGeometry=true&where=%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27%20AND%20%271%27%20%3D%20%271%27&outSR=4326&outFields=*&inSr=4326&geometry=%7B%22xmin%22%3A-126.5625%2C%22ymin%22%3A36.59788913307022%2C%22xmax%22%3A-123.75%2C%22ymax%22%3A38.82259097617713%2C%22spatialReference%22%3A%7B%22wkid%22%3A4326%7D%7D&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelIntersects&geometryPrecision=6&resultType=tile&f=json

Which has 2329 characters.

[gavinr]

esri-leaflet does call encodeURIComponent here, but it seems like it does not encode ' as well as some other characters:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent#Description

Which in your case if the browser does escape those characters, the calculated url length will not be correct. I agree with you that this is probably an issue. I'm not quite sure what to use if not encodeURIComponent though.

If all browsers encoded the apostrophe, we could just replace it with .replace(/'/g, "%27"); , but I'm not sure that's the case. Interesting comment here:

You may be wondering why Chrome converts ' to %27 but encodeURIComponent does not. It's because Chrome implements the up-to-date RFC 3986 definition, which includes ' as a reserved character which must be encoded, but encodeURIComponent uses the outdated RFC 2396 definition instead, which does not list ' as a reserved character.

https://stackoverflow.com/a/48114369/2039

[Voileexperiments]

I think escaping ' and other characters according to RFC 3986 is perfectly fine: it's better to overestimate request URL length than to underestimate it. In the former case you're just making POST requests instead, which is not a big deal at all; while in the latter case it can cause the request to fail completely. I'd prefer making a bit more POST requests than to have no features rendered on the map because the request failed for dumb reasons.

p.s The request is failing completely in this case because it's apparently common to limit maximum URL length to 2048 characters:

Although officially there is no limit specified by RFC 2616, many security protocols and recommendations state that maxQueryStrings on a server should be set to a maximum character limit of 1024. While the entire URL, including the querystring, should be set to a max of 2048 characters. This is to prevent the Slow HTTP Request DDOS vulnerability on a web server. This typically shows up as a vulnerability on the Qualys Web Application Scanner and other security scanners.

https://stackoverflow.com/a/48230425

[gavinr]

I'd prefer making a bit more POST requests than to have no features rendered on the map because the request failed for dumb reasons.

That seems reasonable to me - thanks for sharing your perspective. Unless any other maintainers have concerns, I'd be willing to accept a PR that implements the apostrophe encoding in the length calculation.

[gavinr-maps]

Fixed in v3.0.12.