From e998a95b96b9f04d803f6cd5a202985ab70c0215 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 31 May 2023 23:04:09 -0700 Subject: [PATCH 1/6] d2ir: prevent illegal non-tail keywords --- d2compiler/compile_test.go | 7 +++++++ d2graph/d2graph.go | 3 ++- d2ir/d2ir.go | 7 +++---- .../TestCompile/keyword-container.exp.json | 12 ++++++++++++ 4 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 testdata/d2compiler/TestCompile/keyword-container.exp.json diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 657d095759..cbe2487732 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1722,6 +1722,13 @@ y -> x.style `, expErr: `d2/testdata/d2compiler/TestCompile/edge_to_style.d2:2:8: reserved keywords are prohibited in edges`, }, + { + name: "keyword-container", + + text: `a.near.b +`, + expErr: `d2/testdata/d2compiler/TestCompile/keyword-container.d2:1:3: "near" must be the last part of the key`, + }, { name: "escaped_id", diff --git a/d2graph/d2graph.go b/d2graph/d2graph.go index 301cc5bd86..6a0bfd51be 100644 --- a/d2graph/d2graph.go +++ b/d2graph/d2graph.go @@ -1645,11 +1645,12 @@ var SimpleReservedKeywords = map[string]struct{}{ "classes": {}, } -// ReservedKeywordHolders are reserved keywords that are meaningless on its own and exist solely to hold a set of reserved keywords +// ReservedKeywordHolders are reserved keywords that can hold composites var ReservedKeywordHolders = map[string]struct{}{ "style": {}, "source-arrowhead": {}, "target-arrowhead": {}, + "classes": {}, } // StyleKeywords are reserved keywords which cannot exist outside of the "style" keyword diff --git a/d2ir/d2ir.go b/d2ir/d2ir.go index 50fb8c1049..6f0ad2c672 100644 --- a/d2ir/d2ir.go +++ b/d2ir/d2ir.go @@ -669,10 +669,9 @@ func (m *Map) ensureField(i int, kp *d2ast.KeyPath, refctx *RefContext) (*Field, if _, ok := d2graph.ReservedKeywords[strings.ToLower(head)]; ok { head = strings.ToLower(head) - } - - if head == "class" && i < len(kp.Path)-1 { - return nil, d2parser.Errorf(kp.Path[i].Unbox(), `"class" must be the last part of the key`) + if _, ok := d2graph.ReservedKeywordHolders[head]; !ok && i < len(kp.Path)-1 { + return nil, d2parser.Errorf(kp.Path[i].Unbox(), fmt.Sprintf(`"%s" must be the last part of the key`, head)) + } } if head == "_" { diff --git a/testdata/d2compiler/TestCompile/keyword-container.exp.json b/testdata/d2compiler/TestCompile/keyword-container.exp.json new file mode 100644 index 0000000000..898e9071a8 --- /dev/null +++ b/testdata/d2compiler/TestCompile/keyword-container.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/keyword-container.d2,0:2:2-0:6:6", + "errmsg": "d2/testdata/d2compiler/TestCompile/keyword-container.d2:1:3: \"near\" must be the last part of the key" + } + ] + } +} From febd237f34c245950b28c7a8ba2f20977cad1285 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 31 May 2023 23:06:13 -0700 Subject: [PATCH 2/6] changelog --- ci/release/changelogs/next.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index 396a9e5f2f..09c52375e3 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -15,3 +15,4 @@ - Fixes grid layout overwriting label placements for nested objects. [#1345](https://github.com/terrastruct/d2/pull/1345) - Fixes fonts not rendering correctly on certain platforms. Thanks @mikeday for identifying the solution. [#1356](https://github.com/terrastruct/d2/pull/1356) - Fixes folders not rendering in animations (`--animate-interval`) [#1357](https://github.com/terrastruct/d2/pull/1357) +- Fixes panic using reserved keywords as containers [#1358](https://github.com/terrastruct/d2/pull/1358) From 69f40ae9fb4f0d6a9b0c3177f4a00e90d2adb6f2 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 31 May 2023 23:28:33 -0700 Subject: [PATCH 3/6] fix --- d2graph/d2graph.go | 28 ++++++++++------------------ d2ir/d2ir.go | 2 +- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/d2graph/d2graph.go b/d2graph/d2graph.go index 6a0bfd51be..57f135d57f 100644 --- a/d2graph/d2graph.go +++ b/d2graph/d2graph.go @@ -1618,9 +1618,6 @@ func Key(k *d2ast.KeyPath) []string { // All reserved keywords. See init below. var ReservedKeywords map[string]struct{} -// All reserved keywords not including style keywords. -var ReservedKeywords2 map[string]struct{} - // Non Style/Holder keywords. var SimpleReservedKeywords = map[string]struct{}{ "label": {}, @@ -1642,15 +1639,18 @@ var SimpleReservedKeywords = map[string]struct{}{ "vertical-gap": {}, "horizontal-gap": {}, "class": {}, - "classes": {}, } -// ReservedKeywordHolders are reserved keywords that can hold composites +// ReservedKeywordHolders are reserved keywords that are meaningless on its own and must hold composites var ReservedKeywordHolders = map[string]struct{}{ "style": {}, "source-arrowhead": {}, "target-arrowhead": {}, - "classes": {}, +} + +// CompositeReservedKeywords are reserved keywords that can hold composites +var CompositeReservedKeywords = map[string]struct{}{ + "classes": {}, } // StyleKeywords are reserved keywords which cannot exist outside of the "style" keyword @@ -1726,21 +1726,13 @@ func init() { ReservedKeywords[k] = v } for k, v := range ReservedKeywordHolders { - ReservedKeywords[k] = v + CompositeReservedKeywords[k] = v } for k, v := range BoardKeywords { - ReservedKeywords[k] = v - } - - ReservedKeywords2 = make(map[string]struct{}) - for k, v := range SimpleReservedKeywords { - ReservedKeywords2[k] = v - } - for k, v := range ReservedKeywordHolders { - ReservedKeywords2[k] = v + CompositeReservedKeywords[k] = v } - for k, v := range BoardKeywords { - ReservedKeywords2[k] = v + for k, v := range CompositeReservedKeywords { + ReservedKeywords[k] = v } NearConstants = make(map[string]struct{}, len(NearConstantsArray)) diff --git a/d2ir/d2ir.go b/d2ir/d2ir.go index 6f0ad2c672..bf4b6cdd95 100644 --- a/d2ir/d2ir.go +++ b/d2ir/d2ir.go @@ -669,7 +669,7 @@ func (m *Map) ensureField(i int, kp *d2ast.KeyPath, refctx *RefContext) (*Field, if _, ok := d2graph.ReservedKeywords[strings.ToLower(head)]; ok { head = strings.ToLower(head) - if _, ok := d2graph.ReservedKeywordHolders[head]; !ok && i < len(kp.Path)-1 { + if _, ok := d2graph.CompositeReservedKeywords[head]; !ok && i < len(kp.Path)-1 { return nil, d2parser.Errorf(kp.Path[i].Unbox(), fmt.Sprintf(`"%s" must be the last part of the key`, head)) } } From 4d5d9e65ad0f04a158bcbe24aafc87906b46ec00 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 31 May 2023 23:32:22 -0700 Subject: [PATCH 4/6] remove redundant keywords in compression dict --- lib/urlenc/urlenc.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/urlenc/urlenc.go b/lib/urlenc/urlenc.go index a409fbdcd8..a7bc18cd47 100644 --- a/lib/urlenc/urlenc.go +++ b/lib/urlenc/urlenc.go @@ -20,15 +20,9 @@ var compressionDict = "->" + func init() { var common []string - for k := range d2graph.StyleKeywords { - common = append(common, k) - } for k := range d2graph.ReservedKeywords { common = append(common, k) } - for k := range d2graph.ReservedKeywordHolders { - common = append(common, k) - } sort.Strings(common) for _, k := range common { compressionDict += k From 25fd96c0fcdffafed32651337295ca6402914e3c Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 1 Jun 2023 00:07:57 -0700 Subject: [PATCH 5/6] add testdata for urlenc --- lib/urlenc/testdata/TestBasic.exp.txt | 1 + lib/urlenc/urlenc_test.go | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 lib/urlenc/testdata/TestBasic.exp.txt diff --git a/lib/urlenc/testdata/TestBasic.exp.txt b/lib/urlenc/testdata/TestBasic.exp.txt new file mode 100644 index 0000000000..14ce0930b6 --- /dev/null +++ b/lib/urlenc/testdata/TestBasic.exp.txt @@ -0,0 +1 @@ +FMvBDcIwDEDRe6b4GYAFeuDOHbEAcmKjFFc4VbEQuyMGeG9OZ7JceOwxaf7qPlmTQ30Im9rw8E0TbwxrUmtd-BSIhesNC-4qhz07fV__nqmCpET5ll8AAAD__w== \ No newline at end of file diff --git a/lib/urlenc/urlenc_test.go b/lib/urlenc/urlenc_test.go index c5b9a4866a..21c33087d3 100644 --- a/lib/urlenc/urlenc_test.go +++ b/lib/urlenc/urlenc_test.go @@ -16,6 +16,10 @@ I just forgot my whole philosophy of life!!!: { encoded, err := Encode(script) assert.Success(t, err) + // Make it explicit in PRs when encoding changes + // Something we might want to know for playground compatability + assert.Testdata(t, ".txt", []byte(encoded)) + decoded, err := Decode(encoded) assert.Success(t, err) From 8edf2987bc59dd54a0076b75da80e5314da6b42b Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 1 Jun 2023 00:13:40 -0700 Subject: [PATCH 6/6] test --- lib/urlenc/testdata/TestBasic.exp.txt | 1 - lib/urlenc/testdata/TestChanges.exp.txt | 1 + lib/urlenc/urlenc_test.go | 165 +++++++++++++++++++++++- 3 files changed, 164 insertions(+), 3 deletions(-) delete mode 100644 lib/urlenc/testdata/TestBasic.exp.txt create mode 100644 lib/urlenc/testdata/TestChanges.exp.txt diff --git a/lib/urlenc/testdata/TestBasic.exp.txt b/lib/urlenc/testdata/TestBasic.exp.txt deleted file mode 100644 index 14ce0930b6..0000000000 --- a/lib/urlenc/testdata/TestBasic.exp.txt +++ /dev/null @@ -1 +0,0 @@ -FMvBDcIwDEDRe6b4GYAFeuDOHbEAcmKjFFc4VbEQuyMGeG9OZ7JceOwxaf7qPlmTQ30Im9rw8E0TbwxrUmtd-BSIhesNC-4qhz07fV__nqmCpET5ll8AAAD__w== \ No newline at end of file diff --git a/lib/urlenc/testdata/TestChanges.exp.txt b/lib/urlenc/testdata/TestChanges.exp.txt new file mode 100644 index 0000000000..c8aeb9aed3 --- /dev/null +++ b/lib/urlenc/testdata/TestChanges.exp.txt @@ -0,0 +1 @@ +vFdtb9s4Ev6uXzFwWvQuiN5sN0mFwx1yadMUaLHe2rv9UmBBiyOZrUSqJGUn3ea_L4aSbFl2usUusPkQkzPkzMNnXkhZUSIpoBR3qBMYjeB3DwDvqoJJRkwm8K3kHgDAyQmcni72NpyeOo0Pb-QnTC0wbs5gs1K-VX6mikJtzkDJpWKaC5m3a6-VXKM2zjqUitcFdppaG6WFzM8qlovGf6tabJBZ4MjrqhBpXzNHvUYOnFkGhcrzxtE378GboaoKBC5Mqtao7xMYDUXwURrUa5HiyGO8JeGKN6dzVDiiA0qZBEYnaczG2WTkPXje7lzQmkhg9NNO2LO8zzL4_4UhjiNLDu0fWdRi9lJKRyFRR9sY5ppVqy-FB5CqskRpPQBbGFYJ76G3PmjXJfCaBj-__ShvkKOmyoO51cwquHbl0NBB6ZqAKFlOUaPkT2BlbWWSMKyrQjEebMRnUSIXLFA6D2lW0SwkHEqa0K7qchnGYXwRtj5_e6tyFZh1HsbjKKru_KE8qGS-D7s9VJeeJyeUImjD2qAGWofSwuqea5crZ7AWRixFIew9ZKKwSGnmfetbbMhJYPF27l_N3sC_lkjkc6w0pkTHv4_QRkHYD8swSv_x4cCJZzeCWhFklDSLdnKjHWwOTQD3uaWZCSxqzYzVdWqJgdCoVLAijOKJ35r0Y-JrGKmHnkeCfCz-YrZSEmGDy8cXdxzdLhYzuJJcK8G9DS4T-IDLH0bNvtYaww-4fDqO5k1uPx1H19SHw6uq2pO-VCUT0hw_lPemIsxJz_OzzrUxF8H668Y5FCZ0W8Jf0bXuD0JjgcaErKoK9IWz4sdj_3yaL_2q1iR9PoniC7_8JONJUfjsf084Eia_yJ88O4TSkpEcISHlchqQKBPUiB0ialcNO6Epmba-g2B8i-lKqkLlAk34PB6HrLHb6F0ZHLKwwaXLw23UdjloUqVdY-8a90c5d6LRke6WZRyzmLrbSpUImsnPtPeWJu_dxOtZ7pre9kqYP9qmDuAsVlpkFt7Pro-s3nO-Q9m7CROYDsQEP4qiV5OXBP97CFqQzknb8G-3Y-fu5NGmf7z--6b-EtydgSNkDbR_n5zHvLXElEyumLVMwjiBd93Ey8VXuqvTzwm87oZe034cFQnM3aS5dzxLzfi-EpQfbggzgQPXPU8DzdbZQN7zNzxG569tCYN6aKtzIK00cuHei93pxwnMdsI2of_84ku59F0x-5XMg6xglmau0KmGz6fnsft3Gb9obzKHvqsGl39NWX63KreE9QNTYpmydIUJvGtHR1oQj7-cZxHD4i6qlA7SQtU8o-smkGjDShPZ1i9UrkzIMZrGz6OpH3G-9KeT7NxfXr5AP8NoOo0macouud955e15vAxtukrghn56h-gewAlYXeOOR3rhNtuYrTXSRjf48a1EnpB54oiji_qHdzqolAqt8w6Ey7DGmHckN_6p1GghEJpDEDvw2140DlLdvDz6WbVXYPQoPNRuE2c0GuqOVEb_Ttg2on6L2qkOoSVwzSQXnFlsUmTXSsfbF-tjLeuwIOiP5Dvr-t22fQ-X31zcXN68ouUP9GnDdHG_FJon8IqG_xea3o-1ZXkCv1jm7teKUbjn9NO-n2sprECTwPVu4j14aNMEgiBAmx7w63RdWt92L1Gvn23bAO0J27j0Zc7WjsLme9F4fwQAAP__ \ No newline at end of file diff --git a/lib/urlenc/urlenc_test.go b/lib/urlenc/urlenc_test.go index 21c33087d3..5b492938f4 100644 --- a/lib/urlenc/urlenc_test.go +++ b/lib/urlenc/urlenc_test.go @@ -16,8 +16,169 @@ I just forgot my whole philosophy of life!!!: { encoded, err := Encode(script) assert.Success(t, err) - // Make it explicit in PRs when encoding changes - // Something we might want to know for playground compatability + decoded, err := Decode(encoded) + assert.Success(t, err) + + assert.String(t, script, decoded) +} + +// TestChanges makes it explicit in PRs when encoding changes +// Something we might want to know for playground compatability +func TestChanges(t *testing.T) { + // Choose something with many keywords and varied text + const script = `timeline mixer: "" { + explanation: |md + ## **Timeline mixer** + - Inject ads, who-to-follow, onboarding + - Conversation module + - Cursoring,pagination + - Tweat deduplication + - Served data logging + | +} +People discovery: "People discovery \nservice" +admixer: Ad mixer { + style.fill: "#c1a2f3" +} + +onboarding service: "Onboarding \nservice" +timeline mixer -> People discovery +timeline mixer -> onboarding service +timeline mixer -> admixer +container0: "" { + graphql + comment + tlsapi +} +container0.graphql: GraphQL\nFederated Strato Column { + shape: image + icon: https://upload.wikimedia.org/wikipedia/commons/thumb/1/17/GraphQL_Logo.svg/1200px-GraphQL_Logo.svg.png +} +container0.comment: |md + ## Tweet/user content hydration, visibility filtering +| +container0.tlsapi: TLS-API (being deprecated) +container0.graphql -> timeline mixer +timeline mixer <- container0.tlsapi +twitter fe: "Twitter Frontend " { + icon: https://icons.terrastruct.com/social/013-twitter-1.svg + shape: image +} +twitter fe -> container0.graphql: iPhone web +twitter fe -> container0.tlsapi: HTTP Android +web: Web { + icon: https://icons.terrastruct.com/azure/Web%20Service%20Color/App%20Service%20Domains.svg + shape: image +} + +Iphone: { + icon: 'https://ss7.vzw.com/is/image/VerizonWireless/apple-iphone-12-64gb-purple-53017-mjn13ll-a?$device-lg$' + shape: image +} +Android: { + icon: https://cdn4.iconfinder.com/data/icons/smart-phones-technologies/512/android-phone.png + shape: image +} + +web -> twitter fe +timeline scorer: "Timeline\nScorer" { + style.fill: "#ffdef1" +} +home ranker: Home Ranker + +timeline service: Timeline Service +timeline mixer -> timeline scorer: Thrift RPC +timeline mixer -> home ranker: { + style.stroke-dash: 4 + style.stroke: "#000E3D" +} +timeline mixer -> timeline service +home mixer: Home mixer { + # style.fill: "#c1a2f3" +} +container0.graphql -> home mixer: { + style.stroke-dash: 4 + style.stroke: "#000E3D" +} +home mixer -> timeline scorer +home mixer -> home ranker: { + style.stroke-dash: 4 + style.stroke: "#000E3D" +} +home mixer -> timeline service +manhattan 2: Manhattan +gizmoduck: Gizmoduck +socialgraph: Social graph +tweetypie: Tweety Pie +home mixer -> manhattan 2 +home mixer -> gizmoduck +home mixer -> socialgraph +home mixer -> tweetypie +Iphone -> twitter fe +Android -> twitter fe +prediction service2: Prediction Service { + shape: image + icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png +} +home scorer: Home Scorer { + style.fill: "#ffdef1" +} +manhattan: Manhattan +memcache: Memcache { + icon: https://d1q6f0aelx0por.cloudfront.net/product-logos/de041504-0ddb-43f6-b89e-fe04403cca8d-memcached.png +} + +fetch: Fetch { + style.multiple: true + shape: step +} + +feature: Feature { + style.multiple: true + shape: step +} +scoring: Scoring { + style.multiple: true + shape: step +} +fetch -> feature +feature -> scoring + +prediction service: Prediction Service { + shape: image + icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png +} +scoring -> prediction service +fetch -> container2.crmixer + +home scorer -> manhattan: "" + +home scorer -> memcache: "" +home scorer -> prediction service2 +home ranker -> home scorer +home ranker -> container2.crmixer: Candidate Fetch +container2: "" { + style.stroke: "#000E3D" + style.fill: "#ffffff" + crmixer: CrMixer { + style.fill: "#F7F8FE" + } + earlybird: EarlyBird + utag: Utag + space: Space + communities: Communities +} +etc: ...etc + +home scorer -> etc: Feature Hydration + +feature -> manhattan +feature -> memcache +feature -> etc: Candidate sources +` + + encoded, err := Encode(script) + assert.Success(t, err) assert.Testdata(t, ".txt", []byte(encoded)) decoded, err := Decode(encoded)