Skip to content

Commit 815bde5

Browse files
authored
Merge pull request #1614 from microsoft/paullizer-classic-map-attribution-text
Show inline map attribution as text and keep map tiles on the tile proxy (0.261.049)
2 parents ea53727 + 6b8ac18 commit 815bde5

5 files changed

Lines changed: 196 additions & 4 deletions

File tree

‎application/single_app/config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@
9898
EXECUTOR_TYPE = 'thread'
9999
EXECUTOR_MAX_WORKERS = 30
100100
SESSION_TYPE = 'filesystem'
101-
VERSION = "0.261.048"
101+
VERSION = "0.261.049"
102102
IS_DEVELOPMENT = is_development_env_enabled()
103103

104104
# Opt-out for deployments where App Service Easy Auth is active but the platform

‎application/single_app/static/js/chat/chat-inline-maps.js‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { fetchAgentCitationArtifact } from "./chat-citations.js";
33
import { escapeHtml } from "./chat-utils.js";
44

55
const AZURE_MAPS_RENDER_TYPE = "azure_maps_openlayers";
6+
const AZURE_MAPS_TILE_PROXY_PATH = "/api/azure-maps/tile";
67

78
function toFiniteNumber(value) {
89
const numericValue = Number(value);
@@ -56,14 +57,22 @@ function getCitationResult(candidate) {
5657
return null;
5758
}
5859

60+
// Tiles are requested with the user's session, so they only ever come from SimpleChat's tile proxy.
61+
function isTileProxyTemplate(template) {
62+
const value = typeof template === "string" ? template.trim() : "";
63+
return value.startsWith(`${AZURE_MAPS_TILE_PROXY_PATH}?`)
64+
&& !/[\s"'<>\\]/.test(value)
65+
&& ["{z}", "{x}", "{y}"].every((placeholder) => value.includes(placeholder));
66+
}
67+
5968
function isAzureMapsVisualization(result) {
6069
return Boolean(
6170
result
6271
&& result.success !== false
6372
&& result.render_type === AZURE_MAPS_RENDER_TYPE
6473
&& result.map_payload
6574
&& typeof result.map_payload === "object"
66-
&& result.map_payload.tile_url_template
75+
&& isTileProxyTemplate(result.map_payload.tile_url_template)
6776
);
6877
}
6978

@@ -389,8 +398,9 @@ function initializeOpenLayersMap(mapElement, popupElement, payload) {
389398

390399
const tileLayer = new olRef.layer.Tile({
391400
source: new olRef.source.XYZ({
392-
url: payload.tile_url_template,
393-
attributions: payload.tile_attribution || "",
401+
url: payload.tile_url_template.trim(),
402+
// OpenLayers renders attributions as HTML, and this text comes from a tool result.
403+
attributions: escapeHtml(payload.tile_attribution || ""),
394404
crossOrigin: "anonymous",
395405
maxZoom: payload.view?.max_zoom || 15,
396406
}),
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Inline Map Attribution Text Fix
2+
3+
Fixed/Implemented in version: **0.261.049**
4+
5+
## Issue Description
6+
7+
The chat's inline map card passed a tool result's `map_payload.tile_attribution` straight to
8+
OpenLayers, which renders attribution strings as HTML. The card also drew any tool result shaped
9+
like a map, whatever its `tile_url_template` named. A result from a tool other than the Azure
10+
Maps action could therefore place markup in the map's attribution, or make the browser load map
11+
tiles from another host.
12+
13+
## Root Cause Analysis
14+
15+
- `initializeOpenLayersMap` set `attributions: payload.tile_attribution`, and OpenLayers'
16+
Attribution control writes each attribution string into the page as HTML.
17+
- `isAzureMapsVisualization` only checked that `tile_url_template` was present, not that it
18+
pointed at SimpleChat's tile proxy.
19+
- The Azure Maps action itself always returns a fixed attribution and a proxied tile template,
20+
so maps from that action were never affected.
21+
22+
## Version Implemented
23+
24+
- **0.261.049**
25+
- Application version updated in `application/single_app/config.py` to `0.261.049`.
26+
27+
## Files Modified
28+
29+
- `application/single_app/static/js/chat/chat-inline-maps.js`
30+
- `application/single_app/config.py`
31+
- `functional_tests/test_chat_inline_map_attribution_text.py`
32+
- `docs/explanation/release_notes.md`
33+
34+
## Code Changes Summary
35+
36+
- The attribution is passed through the existing `escapeHtml` helper before OpenLayers receives
37+
it, so it is shown as text.
38+
- New `isTileProxyTemplate` accepts only templates that start with `/api/azure-maps/tile?`,
39+
contain `{z}`, `{x}` and `{y}`, and have no whitespace, quotes, angle brackets or backslashes.
40+
`isAzureMapsVisualization` requires it, so a result naming any other tile source is not drawn.
41+
42+
## Testing Approach
43+
44+
- `functional_tests/test_chat_inline_map_attribution_text.py` checks that the attribution is
45+
escaped and that the map check requires the proxy template. It then runs the real
46+
`isTileProxyTemplate` function in Node against proxy, foreign, protocol-relative, incomplete and
47+
quoted templates.
48+
- `ui_tests/test_chat_inline_azure_maps_rendering.py` already uses a proxied template, so the
49+
existing rendering coverage still applies.
50+
51+
## Impact Analysis
52+
53+
- Maps from the Azure Maps action render exactly as before; the escaped attribution reads the
54+
same.
55+
- A tool result that only resembles a map, with a tile template outside the proxy, is no longer
56+
drawn as a map. Its citation is still available through the normal citation controls.
57+
58+
## Validation
59+
60+
- Before: attribution text from a tool result reached the page as HTML, and tiles could load from
61+
any template.
62+
- After: the attribution is text, and tiles only load through `/api/azure-maps/tile`.

‎docs/explanation/release_notes.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes).
44

5+
### **(v0.261.049)**
6+
7+
#### Bug Fixes
8+
9+
* **Inline Map Attribution Shown as Text and Tiles Kept on the Tile Proxy**
10+
* Inline maps in the chat now show a tool result's tile attribution as plain text. OpenLayers renders attribution strings as HTML, so the text is escaped first.
11+
* A tool result is drawn as a map only when its tile template points at SimpleChat's Azure Maps tile proxy (`/api/azure-maps/tile`). Maps from the Azure Maps action are unchanged.
12+
* (Ref: `chat-inline-maps.js`, [Inline Map Attribution Text Fix](fixes/INLINE_MAP_ATTRIBUTION_TEXT_FIX.md))
13+
514
### **(v0.261.048)**
615

716
#### Bug Fixes
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
#!/usr/bin/env python3
2+
# test_chat_inline_map_attribution_text.py
3+
"""
4+
Functional test for the inline map attribution text fix.
5+
Version: 0.261.049
6+
Implemented in: 0.261.049
7+
8+
This test ensures that the chat's inline map card shows a tool result's tile attribution as
9+
text rather than HTML, and draws a tool result as a map only when its tile template points at
10+
SimpleChat's Azure Maps tile proxy.
11+
"""
12+
13+
import json
14+
import re
15+
import subprocess
16+
import sys
17+
from pathlib import Path
18+
19+
ROOT = Path(__file__).resolve().parents[1]
20+
INLINE_MAPS_JS = ROOT / "application" / "single_app" / "static" / "js" / "chat" / "chat-inline-maps.js"
21+
22+
sys.path.insert(0, str(ROOT / "functional_tests"))
23+
24+
from test_support.versioning import assert_app_version_at_least # noqa: E402
25+
26+
IMPLEMENTED_IN = "0.261.049"
27+
28+
29+
def _source():
30+
return INLINE_MAPS_JS.read_text(encoding="utf-8")
31+
32+
33+
def test_attribution_is_escaped_before_openlayers_renders_it():
34+
"""OpenLayers writes attributions as HTML, so the tool result's text is escaped first."""
35+
print("Testing inline map attribution escaping...")
36+
source = _source()
37+
38+
assert 'import { escapeHtml } from "./chat-utils.js";' in source
39+
assert 'attributions: escapeHtml(payload.tile_attribution || ""),' in source
40+
assert "attributions: payload.tile_attribution" not in source
41+
print("Attribution escaping passed!")
42+
43+
44+
def test_maps_require_the_tile_proxy_template():
45+
"""A tool result is drawn as a map only when its tiles come through the tile proxy."""
46+
print("Testing the tile template requirement...")
47+
source = _source()
48+
49+
assert 'const AZURE_MAPS_TILE_PROXY_PATH = "/api/azure-maps/tile";' in source
50+
assert "&& isTileProxyTemplate(result.map_payload.tile_url_template)" in source
51+
assert "&& result.map_payload.tile_url_template\n" not in source
52+
print("Tile template requirement passed!")
53+
54+
55+
def test_tile_proxy_template_check_behaves():
56+
"""Run the real isTileProxyTemplate function in Node against accepted and refused templates."""
57+
print("Testing isTileProxyTemplate in Node...")
58+
source = _source()
59+
60+
match = re.search(r"function isTileProxyTemplate\(template\) \{.*?\n\}\n", source, re.S)
61+
assert match, "isTileProxyTemplate was not found"
62+
63+
proxy = (
64+
"/api/azure-maps/tile?token=abc&api-version=2024-04-01&tilesetId=microsoft.base.road"
65+
"&zoom={z}&x={x}&y={y}&tileSize=256"
66+
)
67+
cases = {
68+
proxy: True,
69+
f" {proxy} ": True,
70+
"https://tiles.example.test/{z}/{x}/{y}.png": False,
71+
"//tiles.example.test/api/azure-maps/tile?{z}{x}{y}": False,
72+
"/api/azure-maps/tiles?{z}{x}{y}": False,
73+
"/api/azure-maps/tile?zoom={z}&x={x}": False,
74+
'/api/azure-maps/tile?token="><img src=x>&{z}{x}{y}': False,
75+
"": False,
76+
}
77+
script = (
78+
'const AZURE_MAPS_TILE_PROXY_PATH = "/api/azure-maps/tile";\n'
79+
+ match.group(0)
80+
+ f"const cases = {json.dumps(list(cases.items()))};\n"
81+
+ "const wrong = cases.filter(([value, expected]) => isTileProxyTemplate(value) !== expected);\n"
82+
+ "for (const value of [undefined, null, 42, {}]) { if (isTileProxyTemplate(value)) wrong.push([String(value), false]); }\n"
83+
+ "if (wrong.length) { console.error(JSON.stringify(wrong)); process.exit(1); }\n"
84+
+ "console.log(`${cases.length + 4} template checks passed`);\n"
85+
)
86+
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
87+
assert result.returncode == 0, f"isTileProxyTemplate returned the wrong answer for: {result.stderr}"
88+
print(f" {result.stdout.strip()}")
89+
print("isTileProxyTemplate passed!")
90+
91+
92+
def test_version_was_incremented():
93+
assert_app_version_at_least(IMPLEMENTED_IN)
94+
95+
96+
if __name__ == "__main__":
97+
tests = [
98+
test_attribution_is_escaped_before_openlayers_renders_it,
99+
test_maps_require_the_tile_proxy_template,
100+
test_tile_proxy_template_check_behaves,
101+
test_version_was_incremented,
102+
]
103+
failures = 0
104+
for test in tests:
105+
try:
106+
test()
107+
except Exception as exc: # noqa: BLE001 - report every failing check
108+
failures += 1
109+
print(f"FAILED {test.__name__}: {exc}")
110+
print(f"\n{len(tests) - failures}/{len(tests)} tests passed")
111+
sys.exit(1 if failures else 0)

0 commit comments

Comments
 (0)