From 405f1c9777bff77ce8347ff6d306a2850b5660f4 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Fri, 9 Jun 2023 14:10:04 -0700 Subject: [PATCH 1/4] add test --- d2compiler/compile_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 19f41a60b8..56f5b00885 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1505,6 +1505,11 @@ x -> y: { text: `x: {link: https://not-google.com; tooltip: https://google.com}`, expErr: `d2/testdata/d2compiler/TestCompile/no_url_link_and_url_tooltip_concurrently.d2:1:44: Tooltip cannot be set to URL when link is also set (for security)`, }, + { + name: "url_link_non_url_tooltip_ok", + text: `x: {link: https://not-google.com; tooltip: note: url.ParseRequestURI might see this as a URL}`, + expErr: ``, + }, { name: "url_link_and_not_url_tooltip_concurrently", text: `x: {link: https://google.com; tooltip: hello world}`, From 28be902870eedd23e6857c5fe7937b069e8b0eec Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Fri, 9 Jun 2023 14:07:24 -0700 Subject: [PATCH 2/4] don't consider tooltip value a valid url if host is empty --- d2compiler/compile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/d2compiler/compile.go b/d2compiler/compile.go index 4718636c59..0825166d64 100644 --- a/d2compiler/compile.go +++ b/d2compiler/compile.go @@ -533,8 +533,8 @@ func (c *compiler) compileReserved(attrs *d2graph.Attributes, f *d2ir.Field) { } if attrs.Link != nil && attrs.Tooltip != nil { - _, err := url.ParseRequestURI(attrs.Tooltip.Value) - if err == nil { + u, err := url.ParseRequestURI(attrs.Tooltip.Value) + if err == nil && u.Host != "" { c.errorf(scalar, "Tooltip cannot be set to URL when link is also set (for security)") } } From 324a38d285ac487e6312a9bdbab99415c27e4332 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Fri, 9 Jun 2023 14:11:56 -0700 Subject: [PATCH 3/4] update test --- .../url_link_non_url_tooltip_ok.exp.json | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.exp.json diff --git a/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.exp.json b/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.exp.json new file mode 100644 index 0000000000..55017b2a10 --- /dev/null +++ b/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.exp.json @@ -0,0 +1,184 @@ +{ + "graph": { + "name": "", + "isFolderOnly": false, + "ast": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:93:93", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:93:93", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:1:1", + "value": [ + { + "string": "x", + "raw_string": "x" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:3:3-0:93:93", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:4:4-0:32:32", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:4:4-0:8:8", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:4:4-0:8:8", + "value": [ + { + "string": "link", + "raw_string": "link" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:10:10-0:32:32", + "value": [ + { + "string": "https://not-google.com", + "raw_string": "https://not-google.com" + } + ] + } + } + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:34:34-0:92:92", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:34:34-0:41:41", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:34:34-0:41:41", + "value": [ + { + "string": "tooltip", + "raw_string": "tooltip" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:43:43-0:92:92", + "value": [ + { + "string": "note: url.ParseRequestURI might see this as a URL", + "raw_string": "note: url.ParseRequestURI might see this as a URL" + } + ] + } + } + } + } + ] + } + } + } + } + ] + }, + "root": { + "id": "", + "id_val": "", + "attributes": { + "label": { + "value": "" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + }, + "edges": null, + "objects": [ + { + "id": "x", + "id_val": "x", + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/url_link_non_url_tooltip_ok.d2,0:0:0-0:1:1", + "value": [ + { + "string": "x", + "raw_string": "x" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + } + ], + "attributes": { + "label": { + "value": "x" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "tooltip": { + "value": "note: url.ParseRequestURI might see this as a URL" + }, + "link": { + "value": "https://not-google.com" + }, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + } + ] + }, + "err": null +} From 2f10721d4bb83c43058bee72b38b0bbb7cf878bf Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Fri, 9 Jun 2023 14:14:46 -0700 Subject: [PATCH 4/4] changelog --- ci/release/changelogs/next.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index f3c0d2a774..c221c8128d 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -2,4 +2,6 @@ #### Improvements 🧹 +- Improves compiler's tooltip URL check. [#1390](https://github.com/terrastruct/d2/pull/1390) + #### Bugfixes ⛑️