Fix three bugs found reviewing this session's changes

- notes.js's ICON_LABELS wasn't extended for the shop_supermarket/
  shop_convenience keys icons.js added — the note icon picker showed
  the raw internal key as the title instead of a real label.
- Plugins.run mislabeled a malformed plugin.json as "plugin did not
  print valid JSON" — a message written for bad script stdout, not a
  bad manifest, since both paths shared one rescue.
- overpass_pois.rb's description_for joined raw OSM tag values with
  "\n" — an operator/opening_hours tag with an embedded newline could
  inject an extra line into the popup that gpx.js's formatDescription
  (built for p4n2gpx's own well-formed lines) would render as its own
  independent, legitimate-looking link. Tag values are stripped of
  embedded newlines before joining.
This commit is contained in:
randy committed 2026-09-09 18:21:47 +02:00
1 parent 7c664578af
commit ea9cd5fc40
5 files changed
+48 -3

No files matched your search

+13 -2
View File
@@ -106,11 +106,22 @@ module Maptool
private_class_method :category_for private_class_method :category_for
def self.description_for(tags) def self.description_for(tags)
lines = DESCRIPTION_TAGS.filter_map { |tag, label| "#{label}: #{tags[tag]}" if tags[tag] } lines = DESCRIPTION_TAGS.filter_map { |tag, label| "#{label}: #{clean_tag(tags[tag])}" if tags[tag] }
website = tags["website"] || tags["contact:website"] website = tags["website"] || tags["contact:website"]
lines << website if website lines << clean_tag(website) if website
lines.empty? ? nil : lines.join("\n") lines.empty? ? nil : lines.join("\n")
end end
private_class_method :description_for private_class_method :description_for
# An OSM tag value is free text from a world-editable map — an embedded
# newline could otherwise inject an extra "line" of its own into the
# description, e.g. an operator tag ending in "\nhttps://evil.example"
# would render as its own separate, legitimate-looking link line once
# gpx.js's formatDescription (written for p4n2gpx's own well-formed
# "Label: value" lines) splits on "\n".
def self.clean_tag(value)
value.to_s.tr("\n\r", " ").strip
end
private_class_method :clean_tag
end end
end end
+5 -1
View File
@@ -55,7 +55,11 @@ module Maptool
manifest_path = File.join(plugin_dir, "plugin.json") manifest_path = File.join(plugin_dir, "plugin.json")
command = JSON.parse(File.read(manifest_path))["command"] command = begin
JSON.parse(File.read(manifest_path))["command"]
rescue JSON::ParserError
raise RunError, "plugin has an invalid plugin.json"
end
raise RunError, "plugin has no command" unless command raise RunError, "plugin has no command" unless command
south, west, north, east = bbox south, west, north, east = bbox
+2
View File
@@ -24,6 +24,8 @@ const ICON_LABELS = {
leisure_fishing: "Fishing", leisure_fishing: "Fishing",
sport_climbing: "Climbing", sport_climbing: "Climbing",
route_hiking: "Hiking", route_hiking: "Hiking",
shop_supermarket: "Supermarket",
shop_convenience: "Convenience store",
}; };
const NEW_LAYER_VALUE = "__new__"; const NEW_LAYER_VALUE = "__new__";
+17
View File
@@ -56,6 +56,23 @@ class OverpassPoisTest < Minitest::Test
assert_equal "Opening hours: 24/7\nOperator: Town Council\nhttps://example.com", description assert_equal "Opening hours: 24/7\nOperator: Town Council\nhttps://example.com", description
end end
def test_to_geojson_strips_embedded_newlines_from_tag_values
response = {
"elements" => [
{
"type" => "node",
"lat" => 48.1,
"lon" => 17.2,
"tags" => { "amenity" => "toilets", "operator" => "Acme Co\nhttps://evil.example" },
},
],
}
description = Maptool::OverpassPois.to_geojson(response)["features"].first["properties"]["description"]
assert_equal "Operator: Acme Co https://evil.example", description, "an embedded newline must not inject a second, independently-rendered line"
end
def test_to_geojson_omits_description_when_no_recognized_tags_are_present def test_to_geojson_omits_description_when_no_recognized_tags_are_present
response = { "elements" => [{ "type" => "node", "lat" => 48.1, "lon" => 17.2, "tags" => { "amenity" => "toilets" } }] } response = { "elements" => [{ "type" => "node", "lat" => 48.1, "lon" => 17.2, "tags" => { "amenity" => "toilets" } }] }
+11
View File
@@ -53,6 +53,17 @@ class PluginsTest < Minitest::Test
assert_raises(Maptool::Plugins::RunError) { Maptool::Plugins.run(@dir, "hello", [1, 2, 3, 4]) } assert_raises(Maptool::Plugins::RunError) { Maptool::Plugins.run(@dir, "hello", [1, 2, 3, 4]) }
end end
# Distinct from the "did not print valid JSON" message above — that one's
# about the script's stdout, this is about plugin.json itself, before the
# script is ever run. Same exception class either way, but a clear
# message matters since this is what a "Failed" plugin run shows the user
# (see public/js/plugins.js).
def test_run_raises_a_distinct_message_for_an_invalid_manifest
File.write(File.join(@plugin_dir, "plugin.json"), "{not valid json")
error = assert_raises(Maptool::Plugins::RunError) { Maptool::Plugins.run(@dir, "hello", [1, 2, 3, 4]) }
assert_match(/invalid plugin\.json/, error.message)
end
# Multiple dirs (app.rb's PLUGINS_DIRS: the built-in plugins/ plus an # Multiple dirs (app.rb's PLUGINS_DIRS: the built-in plugins/ plus an
# optional private one under data/plugins/) — list/run both accept an # optional private one under data/plugins/) — list/run both accept an
# array, not just the single-dir form the tests above exercise. # array, not just the single-dir form the tests above exercise.