Skip to content

Commit 26ace6b

Browse files
committed
feat: strip relative links to files outside the manifest
Relative links pointing to files not included in the manifest produce broken links in the output. This change detects such links and strips them — keeping only the label text and removing the URL — rather than writing a broken relative path to the destination. Two cases are covered: - Relative link resolves in the source repo but the target file is not in the manifest (resolveDestinationNode returns nil) - Relative link cannot be resolved in the source repo at all (ErrResourceNotFound from ResolveRelativeLink) Absolute links that are not in the manifest continue to pass through unchanged, preserving links to external documentation.
1 parent d2c3760 commit 26ace6b

5 files changed

Lines changed: 83 additions & 16 deletions

File tree

pkg/nodeplugins/markdown/document/document_worker.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,11 @@ func (d *linkResolverTask) resolveLink(dest string, isEmbeddable bool) (string,
167167
return dest, nil
168168
}
169169
}
170-
return d.linkresolver.ResolveResourceLink(dest, d.node, d.source)
170+
resolved, err := d.linkresolver.ResolveResourceLink(dest, d.node, d.source)
171+
if err != nil {
172+
return resolved, err
173+
}
174+
return resolved, nil
171175
}
172176

173177
func (d *linkResolverTask) resolveEmbededLink(embeddedLink string, source string) (string, error) {

pkg/nodeplugins/markdown/document/markdown/link_modifier.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ package markdown
77
import (
88
"bufio"
99
"bytes"
10+
"errors"
1011
"fmt"
1112
"io"
1213
"regexp"
1314
"strings"
1415
"sync"
1516

17+
"github.com/gardener/docforge/pkg/nodeplugins/markdown/linkresolver"
1618
"github.com/yuin/goldmark/ast"
1719
extast "github.com/yuin/goldmark/extension/ast"
1820
"github.com/yuin/goldmark/renderer"
@@ -181,6 +183,7 @@ type Renderer struct {
181183
markers []int
182184
emphasis []byte
183185
table bool
186+
linkStack []int
184187
}
185188

186189
// --------------------------- Node Renders
@@ -487,15 +490,27 @@ func (r *Renderer) renderEmphasis(node ast.Node, entering bool) (ast.WalkStatus,
487490

488491
func (r *Renderer) renderLink(node ast.Node, entering bool) (ast.WalkStatus, error) {
489492
if entering {
493+
r.linkStack = append(r.linkStack, r.writer.Len())
490494
_ = r.writer.WriteByte('[')
491495
} else {
492496
n := node.(*ast.Link)
493-
_ = r.writer.WriteByte(']')
494-
_ = r.writer.WriteByte('(')
495497
dest, err := r.linkResolver(string(n.Destination), false)
498+
var stripErr linkresolver.ErrStripLink
499+
if errors.As(err, &stripErr) {
500+
// strip link: remove the leading '[' and keep only the label text
501+
labelStart := r.linkStack[len(r.linkStack)-1] + 1
502+
r.linkStack = r.linkStack[:len(r.linkStack)-1]
503+
label := bytes.Clone(r.writer.Bytes()[labelStart:])
504+
r.writer.Truncate(labelStart - 1)
505+
_, _ = r.writer.Write(label)
506+
return ast.WalkContinue, nil
507+
}
508+
r.linkStack = r.linkStack[:len(r.linkStack)-1]
496509
if err != nil {
497510
return ast.WalkStop, err
498511
}
512+
_ = r.writer.WriteByte(']')
513+
_ = r.writer.WriteByte('(')
499514
wrap := wrapLinkDestination([]byte(dest))
500515
if wrap {
501516
_ = r.writer.WriteByte('<')

pkg/nodeplugins/markdown/document/markdown/link_modifier_test.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"errors"
1010

1111
"github.com/gardener/docforge/pkg/nodeplugins/markdown/document/markdown"
12+
"github.com/gardener/docforge/pkg/nodeplugins/markdown/linkresolver"
1213
. "github.com/onsi/ginkgo"
1314
. "github.com/onsi/gomega"
1415
"github.com/yuin/goldmark/ast"
@@ -139,6 +140,28 @@ var _ = Describe("Links modifier", func() {
139140
Expect(err.Error()).To(ContainSubstring("fake-error"))
140141
})
141142
})
143+
Context("strip link", func() {
144+
BeforeEach(func() {
145+
lr.strip = true
146+
md = "text:\n[guide](../excluded/file.md) for details.\n"
147+
exp = "text:\nguide for details.\n"
148+
})
149+
It("strips the link and keeps the label", func() {
150+
Expect(err).NotTo(HaveOccurred())
151+
Expect(buf.String()).To(Equal(exp))
152+
})
153+
})
154+
Context("strip link with title", func() {
155+
BeforeEach(func() {
156+
lr.strip = true
157+
md = "text:\n[guide](../excluded/file.md \"some title\") for details.\n"
158+
exp = "text:\nguide for details.\n"
159+
})
160+
It("strips the link and title, keeps only label", func() {
161+
Expect(err).NotTo(HaveOccurred())
162+
Expect(buf.String()).To(Equal(exp))
163+
})
164+
})
142165
})
143166
When("Render markdown with images", func() {
144167
BeforeEach(func() {
@@ -233,11 +256,15 @@ var _ = Describe("Links modifier", func() {
233256
})
234257

235258
type linkResolver struct {
236-
dst string
237-
err error
259+
dst string
260+
err error
261+
strip bool
238262
}
239263

240264
// implements markdown.ResolveLink and fakes the result
241-
func (lr *linkResolver) fakeLink(_ string, _ bool) (string, error) {
265+
func (lr *linkResolver) fakeLink(dest string, _ bool) (string, error) {
266+
if lr.strip {
267+
return "", linkresolver.ErrStripLink{Destination: dest}
268+
}
242269
return lr.dst, lr.err
243270
}

pkg/nodeplugins/markdown/linkresolver/link_resolving.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ import (
2121

2222
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -header ../../../../license_prefix.txt
2323

24+
// ErrStripLink is returned by ResolveResourceLink when a relative link points to a file
25+
// not included in the manifest. The renderer should strip the link and keep only the label text.
26+
type ErrStripLink struct {
27+
Destination string
28+
}
29+
30+
func (e ErrStripLink) Error() string {
31+
return "link destination not in manifest: " + e.Destination
32+
}
33+
2434
// Interface resolves links URLs
2535
//
2636
//counterfeiter:generate . Interface
@@ -62,8 +72,9 @@ func (l *LinkResolver) ResolveResourceLink(resourceLink string, node *manifest.N
6272
if strings.HasPrefix(resourceLink, "#") {
6373
return resourceLink, nil
6474
}
75+
wasRelative := repositoryhost.IsRelative(resourceLink)
6576
// handle relative links to resources
66-
if repositoryhost.IsRelative(resourceLink) {
77+
if wasRelative {
6778
var err error
6879
if srcURL, e := l.Repositoryhosts.ResourceURL(source); e == nil {
6980
resourceLink = ReAnchorRootAbsolute(resourceLink, srcURL.GetResourcePath(), l.Hugo.HugoStructuralDirs)
@@ -72,9 +83,8 @@ func (l *LinkResolver) ResolveResourceLink(resourceLink string, node *manifest.N
7283
resourceLink, err = l.Repositoryhosts.ResolveRelativeLink(source, resourceLink)
7384
if err != nil {
7485
if _, ok := err.(repositoryhost.ErrResourceNotFound); ok {
75-
klog.Warningf("failed to validate absolute link for %s from source %s: %v\n", resourceLink, source, err)
76-
// don't process broken link and don't return error
77-
return resourceLink, nil
86+
klog.V(6).Infof("stripping relative link %s from source %s — target not found in repository", resourceLink, source)
87+
return "", ErrStripLink{Destination: resourceLink}
7888
}
7989
return resourceLink, err
8090
}
@@ -86,7 +96,15 @@ func (l *LinkResolver) ResolveResourceLink(resourceLink string, node *manifest.N
8696
destinationResourceURL := destinationResource.ResourceURL()
8797
destinationNode, err := l.resolveDestinationNode(destinationResourceURL, node)
8898
if destinationNode == nil {
89-
return resourceLink, err
99+
if err != nil {
100+
return resourceLink, err
101+
}
102+
if wasRelative {
103+
klog.V(6).Infof("stripping relative link %s (resolved to %s) — not found in manifest", resourceLink, destinationResourceURL)
104+
return "", ErrStripLink{Destination: resourceLink}
105+
}
106+
klog.V(6).Infof("passing through absolute link %s — not found in manifest", destinationResourceURL)
107+
return resourceLink, nil
90108
}
91109

92110
// construct destination from node path

pkg/nodeplugins/markdown/linkresolver/link_resolving_test.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package linkresolver_test
66

77
import (
88
"embed"
9+
"errors"
910
"testing"
1011

1112
_ "embed"
@@ -59,10 +60,11 @@ var _ = Describe("Document link resolving", func() {
5960
node = linkResolver.SourceToNode[source][0]
6061
})
6162

62-
It("Broken links should not return error", func() {
63+
It("Relative links to files not in repository are stripped", func() {
6364
newLink, err := linkResolver.ResolveResourceLink("invalidfoo/bar.md", node, source)
64-
Expect(err).To(Not(HaveOccurred()))
65-
Expect(newLink).To(Equal("https://github.com/gardener/docforge/blob/master/invalidfoo/bar.md"))
65+
var stripErr linkresolver.ErrStripLink
66+
Expect(errors.As(err, &stripErr)).To(BeTrue())
67+
Expect(newLink).To(Equal(""))
6668
})
6769

6870
It("Resolves linking closest source correctly", func() {
@@ -91,8 +93,9 @@ var _ = Describe("Document link resolving", func() {
9193

9294
It("Resolves non-page resource links correctly", func() {
9395
newLink, err := linkResolver.ResolveResourceLink("./non-page.md", node, source)
94-
Expect(err).ToNot(HaveOccurred())
95-
Expect(newLink).To(Equal("https://github.com/gardener/docforge/blob/master/non-page.md"))
96+
var stripErr linkresolver.ErrStripLink
97+
Expect(errors.As(err, &stripErr)).To(BeTrue())
98+
Expect(newLink).To(Equal(""))
9699
})
97100

98101
It("Resolving url with no suitable repository host", func() {

0 commit comments

Comments
 (0)