Skip to content

Commit

Permalink
Merge pull request terrastruct#2118 from alixander/layered-links-not-…
Browse files Browse the repository at this point in the history
…found

d2compiler: disallow invalid board links
  • Loading branch information
alixander authored Sep 25, 2024
2 parents a3fd4ec + 420fa3d commit 643b670
Show file tree
Hide file tree
Showing 15 changed files with 1,956 additions and 2,266 deletions.
3 changes: 2 additions & 1 deletion ci/release/changelogs/next.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@
- CLI: fixes scale flag not being passed to animated SVG outputs [#2071](https://github.com/terrastruct/d2/pull/2071)
- CLI: pptx exports use theme flags correctly [#2099](https://github.com/terrastruct/d2/pull/2099)
- Imports: importing files with url links is fixed [#2105](https://github.com/terrastruct/d2/pull/2105)
- Vars: substitutions consistently appear in the specified order [#216](https://github.com/terrastruct/d2/pull/2116)
- Vars: substitutions consistently appear in the specified order [#2116](https://github.com/terrastruct/d2/pull/2116)
- Composition: linking to invalid boards no longer produces an invalid link [#2118](https://github.com/terrastruct/d2/pull/2118)
8 changes: 8 additions & 0 deletions d2compiler/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package d2compiler
import (
"encoding/xml"
"fmt"
"html"
"io"
"io/fs"
"net/url"
Expand Down Expand Up @@ -1204,7 +1205,14 @@ func (c *compiler) validateBoardLinks(g *d2graph.Graph) {
continue
}

u, err := url.Parse(html.UnescapeString(obj.Link.Value))
isRemote := err == nil && strings.HasPrefix(u.Scheme, "http")
if isRemote {
continue
}

if linkKey.Path[0].Unbox().ScalarString() != "root" {
obj.Link = nil
continue
}

Expand Down
38 changes: 21 additions & 17 deletions d2compiler/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1607,22 +1607,6 @@ b: {
}
},
},
{
name: "path_link",

text: `x: {
link: Overview.Untitled board 7.zzzzz
}
`,
assertions: func(t *testing.T, g *d2graph.Graph) {
if len(g.Objects) != 1 {
t.Fatal(g.Objects)
}
if g.Objects[0].Link.Value != "Overview.Untitled board 7.zzzzz" {
t.Fatal(g.Objects[0].Link.Value)
}
},
},
{
name: "near_constant",

Expand Down Expand Up @@ -2334,13 +2318,33 @@ scenarios: {
},
},
{
name: "link-board-not-found",
name: "link-board-not-found-1",
text: `x.link: layers.x
`,
assertions: func(t *testing.T, g *d2graph.Graph) {
tassert.Equal(t, (*d2graph.Scalar)(nil), g.Objects[0].Link)
},
},
{
name: "link-board-not-found-2",
text: `layers: {
one: {
ping: {
link: two
}
}
two: {
pong: {
link: one
}
}
}
`,
assertions: func(t *testing.T, g *d2graph.Graph) {
tassert.Equal(t, (*d2graph.Scalar)(nil), g.Layers[0].Objects[0].Link)
tassert.Equal(t, (*d2graph.Scalar)(nil), g.Layers[1].Objects[0].Link)
},
},
{
name: "link-board-not-board",
text: `zzz
Expand Down
57 changes: 1 addition & 56 deletions e2etests-cli/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,12 +1033,9 @@ layers: {
name: "watch-ok-link",
serial: true,
run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) {
// This link technically works because D2 interprets it as a URL,
// and on local filesystem, that is whe path where the compilation happens
// to output it to.
writeFile(t, dir, "index.d2", `
a -> b
b.link: cream
b.link: layers.cream
layers: {
cream: {
Expand Down Expand Up @@ -1201,58 +1198,6 @@ layers: {
assert.Success(t, err)
},
},
{
name: "watch-bad-link",
serial: true,
run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) {
// Just verify we don't crash even with a bad link (it's treated as a URL, which users might have locally)
writeFile(t, dir, "index.d2", `
a -> b
b.link: dream
layers: {
cream: {
c -> b
}
}`)
stderr := &stderrWrapper{}
tms := testMain(dir, env, "--watch", "--browser=0", "index.d2")
tms.Stderr = stderr

tms.Start(t, ctx)
defer func() {
// Manually close, since watcher is daemon
err := tms.Signal(ctx, os.Interrupt)
assert.Success(t, err)
}()

// Wait for watch server to spin up and listen
urlRE := regexp.MustCompile(`127.0.0.1:([0-9]+)`)
watchURL, err := waitLogs(ctx, stderr, urlRE)
assert.Success(t, err)
stderr.Reset()

// Start a client
c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://%s/watch", watchURL), nil)
assert.Success(t, err)
defer c.CloseNow()

// Get the link
_, msg, err := c.Read(ctx)
assert.Success(t, err)
aRE := regexp.MustCompile(`href=\\"([^\"]*)\\"`)
match := aRE.FindSubmatch(msg)
assert.Equal(t, 2, len(match))
linkedPath := match[1]

err = getWatchPage(ctx, t, fmt.Sprintf("http://%s/%s", watchURL, linkedPath))
assert.Success(t, err)

successRE := regexp.MustCompile(`broadcasting update to 1 client`)
_, err = waitLogs(ctx, stderr, successRE)
assert.Success(t, err)
},
},
{
name: "watch-imported-file",
serial: true,
Expand Down
Binary file modified e2etests-cli/testdata/TestCLI_E2E/pptx-theme-overrides.exp.pptx
Binary file not shown.
14 changes: 6 additions & 8 deletions e2etests/testdata/regression/icons_on_top/dagre/board.exp.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 643b670

Please sign in to comment.