From 17ae12b3d727e395b8e7fb623c4417d8c24aeb29 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Mon, 27 Jan 2020 15:57:55 -0500 Subject: [PATCH] Address most common places where tag keys or values could exceed the 255 character API limit (close #6817) --- modules/osm/entity.js | 3 ++- modules/services/osm.js | 15 +++++++++++++++ modules/ui/commit.js | 30 ++++++++++++++++------------- modules/ui/fields/access.js | 2 ++ modules/ui/fields/address.js | 2 ++ modules/ui/fields/combo.js | 17 ++++++++++++++-- modules/ui/fields/cycleway.js | 2 ++ modules/ui/fields/input.js | 2 ++ modules/ui/fields/localized.js | 2 ++ modules/ui/fields/maxspeed.js | 2 ++ modules/ui/fields/textarea.js | 3 ++- modules/ui/fields/wikidata.js | 4 ++-- modules/ui/fields/wikipedia.js | 3 ++- modules/ui/raw_member_editor.js | 2 +- modules/ui/raw_membership_editor.js | 4 ++-- modules/ui/raw_tag_editor.js | 10 ++++++---- 16 files changed, 76 insertions(+), 27 deletions(-) diff --git a/modules/osm/entity.js b/modules/osm/entity.js index f7b3595ef..a92680d6f 100644 --- a/modules/osm/entity.js +++ b/modules/osm/entity.js @@ -150,7 +150,8 @@ osmEntity.prototype = { merged[k] = t2; } else if (t1 !== t2) { changed = true; - merged[k] = utilArrayUnion(t1.split(/;\s*/), t2.split(/;\s*/)).join(';'); + merged[k] = utilArrayUnion(t1.split(/;\s*/), t2.split(/;\s*/)).join(';') + .substr(0, 255); // avoid exceeding character limit; see also services/osm.js -> maxCharsForTagValue() } } return changed ? this.update({ tags: merged }) : this; diff --git a/modules/services/osm.js b/modules/services/osm.js index 082a84ea5..52fb389c7 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -529,6 +529,21 @@ export default { }, + maxCharsForTagKey: function() { + return 255; + }, + + + maxCharsForTagValue: function() { + return 255; + }, + + + maxCharsForRelationRole: function() { + return 255; + }, + + entityURL: function(entity) { return urlroot + '/' + entity.type + '/' + entity.osmId(); }, diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 3b758aa5d..3be15faf8 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -52,6 +52,8 @@ export function uiCommit(context) { var osm = context.connection(); if (!osm) return; + var tagCharLimit = osm.maxCharsForTagValue(); + // expire stored comment, hashtags, source after cutoff datetime - #3947 #4899 var commentDate = +context.storage('commentDate') || 0; var currDate = Date.now(); @@ -84,9 +86,9 @@ export function uiCommit(context) { var detected = utilDetect(); tags = { comment: context.storage('comment') || '', - created_by: ('iD ' + context.version).substr(0, 255), - host: detected.host.substr(0, 255), - locale: detected.locale.substr(0, 255) + created_by: ('iD ' + context.version).substr(0, tagCharLimit), + host: detected.host.substr(0, tagCharLimit), + locale: detected.locale.substr(0, tagCharLimit) }; // call findHashtags initially - this will remove stored @@ -118,7 +120,7 @@ export function uiCommit(context) { } }); - tags.source = sources.join(';').substr(0, 255); + tags.source = sources.join(';').substr(0, tagCharLimit); } _changeset = new osmChangeset({ tags: tags }); @@ -127,24 +129,24 @@ export function uiCommit(context) { tags = Object.assign({}, _changeset.tags); // shallow copy // assign tags for imagery used - var imageryUsed = context.history().imageryUsed().join(';').substr(0, 255); + var imageryUsed = context.history().imageryUsed().join(';').substr(0, tagCharLimit); tags.imagery_used = imageryUsed || 'None'; // assign tags for closed issues and notes var osmClosed = osm.getClosedIDs(); if (osmClosed.length) { - tags['closed:note'] = osmClosed.join(';').substr(0, 255); + tags['closed:note'] = osmClosed.join(';').substr(0, tagCharLimit); } if (services.keepRight) { var krClosed = services.keepRight.getClosedIDs(); if (krClosed.length) { - tags['closed:keepright'] = krClosed.join(';').substr(0, 255); + tags['closed:keepright'] = krClosed.join(';').substr(0, tagCharLimit); } } if (services.improveOSM) { var iOsmClosed = services.improveOSM.getClosedIDs(); if (iOsmClosed.length) { - tags['closed:improveosm'] = iOsmClosed.join(';').substr(0, 255); + tags['closed:improveosm'] = iOsmClosed.join(';').substr(0, tagCharLimit); } } @@ -163,10 +165,10 @@ export function uiCommit(context) { var issuesBySubtype = utilArrayGroupBy(issuesOfType, 'subtype'); for (var issueSubtype in issuesBySubtype) { var issuesOfSubtype = issuesBySubtype[issueSubtype]; - tags[prefix + ':' + issueType + ':' + issueSubtype] = issuesOfSubtype.length.toString().substr(0, 255); + tags[prefix + ':' + issueType + ':' + issueSubtype] = issuesOfSubtype.length.toString().substr(0, tagCharLimit); } } else { - tags[prefix + ':' + issueType] = issuesOfType.length.toString().substr(0, 255); + tags[prefix + ':' + issueType] = issuesOfType.length.toString().substr(0, tagCharLimit); } } } @@ -513,16 +515,18 @@ export function uiCommit(context) { function updateChangeset(changed, onInput) { var tags = Object.assign({}, _changeset.tags); // shallow copy + var tagCharLimit = services.osm.maxCharsForTagValue(); + Object.keys(changed).forEach(function(k) { var v = changed[k]; - k = k.trim().substr(0, 255); + k = k.trim().substr(0, tagCharLimit); if (readOnlyTags.indexOf(k) !== -1) return; if (k !== '' && v !== undefined) { if (onInput) { tags[k] = v; } else { - tags[k] = v.trim().substr(0, 255); + tags[k] = v.trim().substr(0, tagCharLimit); } } else { delete tags[k]; @@ -534,7 +538,7 @@ export function uiCommit(context) { var commentOnly = changed.hasOwnProperty('comment') && (changed.comment !== ''); var arr = findHashtags(tags, commentOnly); if (arr.length) { - tags.hashtags = arr.join(';').substr(0, 255); + tags.hashtags = arr.join(';').substr(0, tagCharLimit); context.storage('hashtags', tags.hashtags); } else { delete tags.hashtags; diff --git a/modules/ui/fields/access.js b/modules/ui/fields/access.js index 0c781a7c3..15f902960 100644 --- a/modules/ui/fields/access.js +++ b/modules/ui/fields/access.js @@ -1,6 +1,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; +import { services } from '../../services'; import { uiCombobox } from '../combobox'; import { utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; @@ -46,6 +47,7 @@ export function uiFieldAccess(field, context) { .attr('class', 'preset-input-access-wrap') .append('input') .attr('type', 'text') + .attr('maxlength', services.osm.maxCharsForTagValue()) .attr('class', function(d) { return 'preset-input-access preset-input-access-' + d; }) .call(utilNoAuto) .each(function(d) { diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 8dcbf4244..9425208a7 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -4,6 +4,7 @@ import * as countryCoder from '@ideditor/country-coder'; import { dataAddressFormats } from '../../../data'; import { geoExtent, geoChooseEdge, geoSphericalDistance } from '../../geo'; +import { services } from '../../services'; import { uiCombobox } from '../combobox'; import { utilArrayUniqBy, utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; import { t } from '../../util/locale'; @@ -163,6 +164,7 @@ export function uiFieldAddress(field, context) { var tkey = addrField.strings.placeholders[localkey] ? localkey : d.id; return addrField.t('placeholders.' + tkey); }) + .attr('maxlength', services.osm.maxCharsForTagValue()) .attr('class', function (d) { return 'addr-' + d.id; }) .call(utilNoAuto) .each(addDropdown) diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index c9a49d867..5de03db57 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -359,6 +359,7 @@ export function uiFieldCombo(field, context) { .append('input') .attr('type', 'text') .attr('id', 'preset-input-' + field.safeid) + .attr('maxlength', services.osm.maxCharsForTagValue()) .call(utilNoAuto) .call(initCombo, selection) .merge(input); @@ -400,6 +401,8 @@ export function uiFieldCombo(field, context) { if (isMulti || isSemi) { _multiData = []; + var maxLength; + if (isMulti) { // Build _multiData array containing keys already set.. for (var k in tags) { @@ -417,6 +420,9 @@ export function uiFieldCombo(field, context) { // Set keys for form-field modified (needed for undo and reset buttons).. field.keys = _multiData.map(function(d) { return d.key; }); + // limit the input length so it fits after prepending the key prefix + maxLength = services.osm.maxCharsForTagKey() - field.key.length; + } else if (isSemi) { var arr = utilArrayUniq((tags[field.key] || '').split(';')).filter(Boolean); _multiData = arr.map(function(k) { @@ -425,6 +431,9 @@ export function uiFieldCombo(field, context) { value: displayValue(k) }; }); + + // limit the input length to the remaining available characters + maxLength = services.osm.maxCharsForTagValue() - (tags[field.key] || '').length; } // Exclude existing multikeys from combo options.. @@ -432,9 +441,10 @@ export function uiFieldCombo(field, context) { combobox.data(available); // Hide 'Add' button if this field uses fixed set of - // translateable optstrings and they're all currently used.. + // translateable optstrings and they're all currently used, + // or if the field is already at its character limit container.selectAll('.combobox-input, .combobox-caret') - .classed('hide', optstrings && !available.length); + .classed('hide', (optstrings && !available.length) || !maxLength); // Render chips @@ -467,6 +477,9 @@ export function uiFieldCombo(field, context) { .attr('class', 'remove') .text('×'); + container.selectAll('input[type="text"]') + .attr('maxlength', maxLength); + } else { utilGetSetValue(input, displayValue(tags[field.key])); } diff --git a/modules/ui/fields/cycleway.js b/modules/ui/fields/cycleway.js index 12c097d26..abc833a73 100644 --- a/modules/ui/fields/cycleway.js +++ b/modules/ui/fields/cycleway.js @@ -1,6 +1,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; +import { services } from '../../services'; import { uiCombobox } from '../combobox'; import { utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; @@ -54,6 +55,7 @@ export function uiFieldCycleway(field, context) { .attr('class', 'preset-input-cycleway-wrap') .append('input') .attr('type', 'text') + .attr('maxlength', services.osm.maxCharsForTagValue()) .attr('class', function(d) { return 'preset-input-cycleway preset-input-' + stripcolon(d); }) .call(utilNoAuto) .each(function(d) { diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 1258a0c9a..7a0152702 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -4,6 +4,7 @@ import * as countryCoder from '@ideditor/country-coder'; import { t, textDirection } from '../../util/locale'; import { dataPhoneFormats } from '../../../data'; +import { services } from '../../services'; import { utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; import { svgIcon } from '../../svg/icon'; @@ -45,6 +46,7 @@ export function uiFieldText(field, context) { .attr('type', field.type === 'identifier' ? 'text' : field.type) .attr('id', fieldID) .attr('placeholder', field.placeholder() || t('inspector.unknown')) + .attr('maxlength', services.osm.maxCharsForTagValue()) .classed(field.type, true) .call(utilNoAuto) .merge(input); diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 115632cce..deaad2d38 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -147,6 +147,7 @@ export function uiFieldLocalized(field, context) { .attr('id', 'preset-input-' + field.safeid) .attr('class', 'localized-main') .attr('placeholder', field.placeholder()) + .attr('maxlength', services.osm.maxCharsForTagValue()) .call(utilNoAuto) .merge(input); @@ -510,6 +511,7 @@ export function uiFieldLocalized(field, context) { .append('input') .attr('type', 'text') .attr('placeholder', t('translate.localized_translation_name')) + .attr('maxlength', services.osm.maxCharsForTagValue()) .attr('class', 'localized-value') .on('blur', changeValue) .on('change', changeValue); diff --git a/modules/ui/fields/maxspeed.js b/modules/ui/fields/maxspeed.js index 08881438b..e1c853b1b 100644 --- a/modules/ui/fields/maxspeed.js +++ b/modules/ui/fields/maxspeed.js @@ -2,6 +2,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; import * as countryCoder from '@ideditor/country-coder'; +import { services } from '../../services'; import { uiCombobox } from '../combobox'; import { utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; @@ -39,6 +40,7 @@ export function uiFieldMaxspeed(field, context) { .append('input') .attr('type', 'text') .attr('id', 'preset-input-' + field.safeid) + .attr('maxlength', services.osm.maxCharsForTagValue() - 4) .attr('placeholder', field.placeholder()) .call(utilNoAuto) .call(speedCombo) diff --git a/modules/ui/fields/textarea.js b/modules/ui/fields/textarea.js index d319f2b56..be218e074 100644 --- a/modules/ui/fields/textarea.js +++ b/modules/ui/fields/textarea.js @@ -1,6 +1,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; +import { services } from '../../services'; import { t } from '../../util/locale'; import { utilGetSetValue, @@ -30,7 +31,7 @@ export function uiFieldTextarea(field) { .append('textarea') .attr('id', 'preset-input-' + field.safeid) .attr('placeholder', field.placeholder() || t('inspector.unknown')) - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForTagValue()) .call(utilNoAuto) .on('input', change(true)) .on('blur', change()) diff --git a/modules/ui/fields/wikidata.js b/modules/ui/fields/wikidata.js index 6169b16c7..2668dc8c4 100644 --- a/modules/ui/fields/wikidata.js +++ b/modules/ui/fields/wikidata.js @@ -198,7 +198,7 @@ export function uiFieldWikidata(field, context) { var siteID = lang.replace('-', '_') + 'wiki'; if (entity.sitelinks[siteID]) { foundPreferred = true; - currTags[_wikipediaKey] = lang + ':' + entity.sitelinks[siteID].title; + currTags[_wikipediaKey] = (lang + ':' + entity.sitelinks[siteID].title).substr(0, services.osm.maxCharsForTagValue()); // use the first match break; } @@ -220,7 +220,7 @@ export function uiFieldWikidata(field, context) { } else { var wikiLang = wikiSiteKeys[0].slice(0, -4).replace('_', '-'); var wikiTitle = entity.sitelinks[wikiSiteKeys[0]].title; - currTags[_wikipediaKey] = wikiLang + ':' + wikiTitle; + currTags[_wikipediaKey] = (wikiLang + ':' + wikiTitle).substr(0, services.osm.maxCharsForTagValue()); } } } diff --git a/modules/ui/fields/wikipedia.js b/modules/ui/fields/wikipedia.js index 86cbc353c..af9175cb4 100644 --- a/modules/ui/fields/wikipedia.js +++ b/modules/ui/fields/wikipedia.js @@ -106,6 +106,7 @@ export function uiFieldWikipedia(field, context) { .attr('type', 'text') .attr('class', 'wiki-title') .attr('id', 'preset-input-' + field.safeid) + .attr('maxlength', services.osm.maxCharsForTagValue() - 4) .call(utilNoAuto) .call(titleCombo) .merge(title); @@ -184,7 +185,7 @@ export function uiFieldWikipedia(field, context) { } if (value) { - syncTags.wikipedia = language()[2] + ':' + value; + syncTags.wikipedia = (language()[2] + ':' + value).substr(0, services.osm.maxCharsForTagValue()); } else { syncTags.wikipedia = undefined; } diff --git a/modules/ui/raw_member_editor.js b/modules/ui/raw_member_editor.js index 67d27c282..67a957d94 100644 --- a/modules/ui/raw_member_editor.js +++ b/modules/ui/raw_member_editor.js @@ -209,7 +209,7 @@ export function uiRawMemberEditor(context) { .append('input') .attr('class', 'member-role') .property('type', 'text') - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForRelationRole()) .attr('placeholder', t('inspector.role')) .call(utilNoAuto); diff --git a/modules/ui/raw_membership_editor.js b/modules/ui/raw_membership_editor.js index 0af3fe0ff..d557000ac 100644 --- a/modules/ui/raw_membership_editor.js +++ b/modules/ui/raw_membership_editor.js @@ -248,7 +248,7 @@ export function uiRawMembershipEditor(context) { .append('input') .attr('class', 'member-role') .property('type', 'text') - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForRelationRole()) .attr('placeholder', t('inspector.role')) .call(utilNoAuto) .property('value', function(d) { return d.member.role; }) @@ -296,7 +296,7 @@ export function uiRawMembershipEditor(context) { .append('input') .attr('class', 'member-role') .property('type', 'text') - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForRelationRole()) .attr('placeholder', t('inspector.role')) .call(utilNoAuto); diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index b8aa43ea7..9691ff4f0 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -207,7 +207,7 @@ export function uiRawTagEditor(context) { .append('input') .property('type', 'text') .attr('class', 'key') - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForTagKey()) .call(utilNoAuto) .on('blur', keyChange) .on('change', keyChange); @@ -218,7 +218,7 @@ export function uiRawTagEditor(context) { .append('input') .property('type', 'text') .attr('class', 'value') - .attr('maxlength', 255) + .attr('maxlength', services.osm.maxCharsForTagValue()) .call(utilNoAuto) .on('blur', valueChange) .on('change', valueChange) @@ -362,11 +362,13 @@ export function uiRawTagEditor(context) { function textChanged() { var newText = this.value.trim(); var newTags = {}; + var maxKeyLength = services.osm.maxCharsForTagKey(); + var maxValueLength = services.osm.maxCharsForTagValue(); newText.split('\n').forEach(function(row) { var m = row.match(/^\s*([^=]+)=(.*)$/); if (m !== null) { - var k = unstringify(m[1].trim()); - var v = unstringify(m[2].trim()); + var k = unstringify(m[1].trim()).substr(0, maxKeyLength); + var v = unstringify(m[2].trim()).substr(0, maxValueLength); newTags[k] = v; } });