Skip to content

Commit fc95571

Browse files
committed
Rework inlineSnippet in Renderables
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
1 parent 2938e4b commit fc95571

5 files changed

Lines changed: 226 additions & 25 deletions

File tree

headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Renderables.java

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,28 @@ public void renderAsHtml(HtmlBuffer buffer) {
111111
};
112112
}
113113

114-
public static Renderable inlineSnippet(Renderable text) {
114+
/**
115+
* A single inline code span wrapping a leaf string (code span content is never
116+
* re-parsed as markdown, so there's nothing to gain by accepting a nested
117+
* {@link Renderable} here). Renders as an escaped markdown code span (a backtick
118+
* fence long enough that embedded backticks in {@code content} can't terminate it
119+
* early, padded with a space where CommonMark requires it), or as an HTML
120+
* {@code <code>} element with HTML-entity escaping.
121+
*/
122+
public static Renderable inlineSnippet(String content) {
123+
String safeContent = content == null ? "null" : content;
115124
return new Renderable() {
116125

117126
@Override
118127
public void renderAsMarkdown(StringBuilder buffer) {
119-
buffer.append("`");
120-
text.renderAsMarkdown(buffer);
121-
buffer.append("`");
128+
buffer.append(codeSpanMarkdown(safeContent));
122129
}
123130

124131
@Override
125132
public void renderAsHtml(HtmlBuffer buffer) {
126-
buffer.raw("<pre>");
127-
text.renderAsHtml(buffer);
128-
buffer.raw("</pre>");
133+
buffer.raw("<code>");
134+
buffer.text(safeContent);
135+
buffer.raw("</code>");
129136
}
130137
};
131138
}
@@ -206,7 +213,9 @@ public static Renderable link(String text, String url) {
206213
@Override
207214
public void renderAsMarkdown(StringBuilder buffer) {
208215
buffer.append('[');
209-
buffer.append(text);
216+
// Escape characters that would otherwise prematurely close the link text or
217+
// be misread as markdown syntax (e.g. a bean id/type coming from a live process).
218+
buffer.append(text.replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]"));
210219
buffer.append(']');
211220
if (url != null) {
212221
buffer.append('(');
@@ -227,6 +236,41 @@ public void renderAsHtml(HtmlBuffer buffer) {
227236
};
228237
}
229238

239+
/**
240+
* Wraps {@code content} in a markdown inline code span, choosing a backtick
241+
* fence long enough that embedded backticks in {@code content} can't
242+
* terminate the span early, and padding with a space on each side where
243+
* CommonMark requires it (content that starts/ends with a backtick, or
244+
* that starts and ends with a space but isn't all spaces) so the parser's
245+
* own space-stripping rule doesn't alter {@code content}.
246+
*/
247+
private static String codeSpanMarkdown(String content) {
248+
if (content.isEmpty()) {
249+
// An empty code span can't be represented in markdown (e.g. "``" is not
250+
// parsed as one); omitting it entirely is the safest fallback.
251+
return "";
252+
}
253+
int maxRun = 0;
254+
int run = 0;
255+
boolean allSpaces = true;
256+
for (int i = 0; i < content.length(); i++) {
257+
char c = content.charAt(i);
258+
if (c == '`') {
259+
run++;
260+
maxRun = Math.max(maxRun, run);
261+
} else {
262+
run = 0;
263+
}
264+
if (c != ' ') {
265+
allSpaces = false;
266+
}
267+
}
268+
String fence = "`".repeat(maxRun + 1);
269+
boolean pad = content.startsWith("`") || content.endsWith("`")
270+
|| (content.startsWith(" ") && content.endsWith(" ") && !allSpaces);
271+
return pad ? fence + " " + content + " " + fence : fence + content + fence;
272+
}
273+
230274
public static Renderable lineBreak() {
231275
return new Renderable() {
232276

headless-services/commons/commons-util/src/test/java/org/springframework/ide/vscode/commons/util/RenderablesTest.java

Lines changed: 164 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,25 @@
1515
import org.junit.jupiter.api.Test;
1616

1717
public class RenderablesTest {
18-
18+
19+
private static Renderable snippet(String content) {
20+
return Renderables.inlineSnippet(content);
21+
}
22+
23+
private static String snippetMD(String content) {
24+
return snippet(content).toMarkdown();
25+
}
26+
27+
@Test
28+
void inlineSnippetRendersAsHtmlCodeElementWithEntityEscaping() {
29+
assertThat(snippet("a<b>&c").toHtml()).isEqualTo("<code>a&lt;b&gt;&amp;c</code>");
30+
}
31+
32+
@Test
33+
void inlineSnippetOfNullRendersAsHtmlLiteralNullTooForConsistencyWithMarkdown() {
34+
assertThat(snippet(null).toHtml()).isEqualTo("<code>null</code>");
35+
}
36+
1937
@Test
2038
void escapeParenthesisForMardownLink() {
2139
Renderable r = Renderables.link("my-link-with-parenthesis", "https://foo.com/index(1).html");
@@ -24,4 +42,149 @@ void escapeParenthesisForMardownLink() {
2442
assertThat(sb.toString()).isEqualTo("[my-link-with-parenthesis](https://foo.com/index%281%29.html)");
2543
}
2644

45+
@Test
46+
void escapeBracketsAndBackslashForMarkdownLinkText() {
47+
Renderable r = Renderables.link("weird]name\\with[brackets", "https://foo.com");
48+
StringBuilder sb = new StringBuilder();
49+
r.renderAsMarkdown(sb);
50+
assertThat(sb.toString()).isEqualTo("[weird\\]name\\\\with\\[brackets](https://foo.com)");
51+
}
52+
53+
@Test
54+
void escapeLoneBackslashNotAdjacentToABracketInLinkText() {
55+
// A single '\' between two plain characters: CommonMark only treats "\" followed by
56+
// ASCII punctuation as an escape, so a lone backslash must still be doubled or a
57+
// compliant parser would otherwise just print it literally anyway — doubling it here
58+
// keeps the escaping logic uniform regardless of what follows the backslash.
59+
Renderable r = Renderables.link("a\\b", "https://foo.com");
60+
StringBuilder sb = new StringBuilder();
61+
r.renderAsMarkdown(sb);
62+
assertThat(sb.toString()).isEqualTo("[a\\\\b](https://foo.com)");
63+
}
64+
65+
@Test
66+
void escapeIsAppliedUniformlyEvenWhenInputAlreadyContainsABackslashBracketPair() {
67+
// Guards the escaping ORDER: backslash must be escaped before '[' / ']', otherwise an
68+
// attacker-supplied "\]" could ride through as what looks like an already-escaped
69+
// bracket and end up under-escaped. Build inputs/outputs via explicit char appends
70+
// instead of string literals so the backslash counts can't be miscounted by eye.
71+
String input = new StringBuilder().append('\\').append(']').toString(); // real chars: \ ]
72+
Renderable r = Renderables.link(input, "https://foo.com");
73+
StringBuilder sb = new StringBuilder();
74+
r.renderAsMarkdown(sb);
75+
String expectedLabel = new StringBuilder().append('\\').append('\\').append('\\').append(']').toString(); // real chars: \ \ \ ]
76+
assertThat(sb.toString()).isEqualTo("[" + expectedLabel + "](https://foo.com)");
77+
}
78+
79+
@Test
80+
void escapeAppliesToBalancedBracketsToo() {
81+
// CommonMark link text may legally contain balanced brackets unescaped, but escaping
82+
// them anyway is harmless (an escaped bracket renders as the literal character) and
83+
// avoids having to reason about balance for attacker-controlled content.
84+
Renderable r = Renderables.link("[nested]", "https://foo.com");
85+
StringBuilder sb = new StringBuilder();
86+
r.renderAsMarkdown(sb);
87+
assertThat(sb.toString()).isEqualTo("[\\[nested\\]](https://foo.com)");
88+
}
89+
90+
@Test
91+
void inlineSnippetWithoutBacktick() {
92+
assertThat(snippetMD("plain-text")).isEqualTo("`plain-text`");
93+
}
94+
95+
@Test
96+
void inlineSnippetWithEmbeddedSingleBacktick() {
97+
assertThat(snippetMD("has`backtick")).isEqualTo("``has`backtick``");
98+
}
99+
100+
@Test
101+
void inlineSnippetWithLongerEmbeddedBacktickRun() {
102+
assertThat(snippetMD("has``double")).isEqualTo("```has``double```");
103+
}
104+
105+
@Test
106+
void inlineSnippetFenceScalesToLongestEmbeddedBacktickRun() {
107+
// A run of 3 backticks in the content needs a 4-backtick fence, not just "one more
108+
// than the shortest run" — this pins the fence length to maxRun + 1 in general, not
109+
// just for the 1- and 2-backtick cases already covered above.
110+
assertThat(snippetMD("a```b")).isEqualTo("````a```b````");
111+
}
112+
113+
@Test
114+
void inlineSnippetFenceSizedByLongestOfSeveralDistinctRuns() {
115+
// Runs of length 1, 2, then 3 appear in the content; the fence must be sized off the
116+
// longest (3), not the first or the last one encountered.
117+
assertThat(snippetMD("a`b``c```d")).isEqualTo("````a`b``c```d````");
118+
}
119+
120+
@Test
121+
void inlineSnippetOfSingleBacktickCharacter() {
122+
// The canonical CommonMark example: content that is exactly one backtick needs a
123+
// 2-backtick fence plus padding, since the content both starts and ends with '`'.
124+
assertThat(snippetMD("`")).isEqualTo("`` ` ``");
125+
}
126+
127+
@Test
128+
void inlineSnippetOfContentThatIsEntirelyBackticks() {
129+
assertThat(snippetMD("``")).isEqualTo("``` `` ```");
130+
}
131+
132+
@Test
133+
void inlineSnippetPadsWhenContentStartsOrEndsWithBacktick() {
134+
assertThat(snippetMD("`leading")).isEqualTo("`` `leading ``");
135+
assertThat(snippetMD("trailing`")).isEqualTo("`` trailing` ``");
136+
}
137+
138+
@Test
139+
void inlineSnippetPadsBothStartAndEndBacktickBoundary() {
140+
assertThat(snippetMD("`both`")).isEqualTo("`` `both` ``");
141+
}
142+
143+
@Test
144+
void inlineSnippetOfEmptyString() {
145+
// An empty code span can't be represented in markdown; omit it entirely
146+
// rather than rendering literal spaces (as a naive "` `" wrapping would).
147+
assertThat(snippetMD("")).isEqualTo("");
148+
}
149+
150+
@Test
151+
void inlineSnippetOfNull() {
152+
assertThat(snippetMD(null)).isEqualTo("`null`");
153+
}
154+
155+
@Test
156+
void inlineSnippetPadsWhenContentStartsAndEndsWithSpaceButIsNotAllSpaces() {
157+
// Without the extra pad space, CommonMark's own space-stripping rule would
158+
// turn "` a `" into "a", losing the leading/trailing space from the original content.
159+
assertThat(snippetMD(" a ")).isEqualTo("` a `");
160+
}
161+
162+
@Test
163+
void inlineSnippetPadIsIndependentOfHowManyBoundarySpacesAreAlreadyPresent() {
164+
// The parser only ever strips ONE space from each side, no matter how many are there,
165+
// so exactly one pad space is enough to protect a 2-space boundary too.
166+
assertThat(snippetMD(" a ")).isEqualTo("`" + " " + "a" + " " + "`");
167+
}
168+
169+
@Test
170+
void inlineSnippetDoesNotPadWhenOnlyOneSideHasABoundarySpace() {
171+
// CommonMark's strip rule requires the content to BOTH start and end with a space;
172+
// a space on only one side is left completely alone by the parser, so padding it
173+
// would incorrectly add a space that was never there in the original content.
174+
assertThat(snippetMD(" abc")).isEqualTo("` abc`");
175+
assertThat(snippetMD("abc ")).isEqualTo("`abc `");
176+
}
177+
178+
@Test
179+
void inlineSnippetDoesNotPadContentThatIsEntirelySpaces() {
180+
// CommonMark's space-stripping rule only applies when the content isn't all spaces,
181+
// so no extra padding is needed to preserve an all-space payload.
182+
assertThat(snippetMD(" ")).isEqualTo("` `");
183+
}
184+
185+
@Test
186+
void inlineSnippetOfSingleSpaceIsTreatedAsAllSpaces() {
187+
assertThat(snippetMD(" ")).isEqualTo("` `");
188+
}
189+
27190
}

headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/LiveHoverUtils.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public static String showBean(LiveBean bean) {
5151
StringBuilder buf = new StringBuilder("Bean [id: " + bean.getId());
5252
String type = bean.getType(true);
5353
if (type != null) {
54-
buf.append(", type: `" + type + "`");
54+
buf.append(", type: " + Renderables.inlineSnippet(type).toMarkdown());
5555
}
5656
buf.append(']');
5757
return buf.toString();
@@ -66,9 +66,7 @@ public static String showBeanWithResource(SourceLinks sourceLinks, LiveBean bean
6666
String type = bean.getType(true);
6767

6868
StringBuilder buf = new StringBuilder("Bean: ");
69-
buf.append('`');
70-
buf.append(bean.getId());
71-
buf.append('`');
69+
buf.append(Renderables.inlineSnippet(bean.getId()).toMarkdown());
7270
if (type != null) {
7371
// Try creating a URL link to open source for the type
7472
buf.append(newline);
@@ -77,7 +75,7 @@ public static String showBeanWithResource(SourceLinks sourceLinks, LiveBean bean
7775
if (url.isPresent()) {
7876
buf.append(Renderables.link(type, url.get()).toMarkdown());
7977
} else {
80-
buf.append("`" + type + "`");
78+
buf.append(Renderables.inlineSnippet(type).toMarkdown());
8179
}
8280
}
8381
String resource = bean.getResource();
@@ -106,11 +104,7 @@ public static String showBeanTypeMarkdown(SourceLinks sourceLinks, IJavaProject
106104
return CANT_MATCH_PROPER_BEAN.getId();
107105
} else {
108106
String type = bean.getType(true);
109-
StringBuilder sb = new StringBuilder();
110-
sb.append('`');
111-
sb.append(getShortDisplayType(bean));
112-
sb.append('`');
113-
String displayId = sb.toString();
107+
String displayId = Renderables.inlineSnippet(getShortDisplayType(bean)).toMarkdown();
114108
if (type != null && sourceLinks != null) {
115109
Optional<String> url = sourceLinks.sourceLinkUrlForFQName(project, type);
116110
if (url.isPresent()) {

headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringResource.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2017, 2019 Pivotal, Inc.
2+
* Copyright (c) 2017, 2026 Pivotal, Inc.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -78,14 +78,14 @@ public String toMarkdown() {
7878
}
7979
// not a project relative path
8080
return linkUrl.isPresent() ? Renderables.link(projectRelativePath(project, path), linkUrl.get()).toMarkdown()
81-
: "`" + projectRelativePath(project, path) + "`";
81+
: Renderables.inlineSnippet(projectRelativePath(project, path)).toMarkdown();
8282
case CLASS_PATH_RESOURCE:
8383
int idx = path.lastIndexOf(SourceLinks.CLASS);
8484
if (idx >= 0) {
8585
Path p = Paths.get(path.substring(0, idx));
8686
linkUrl = sourceLinks.sourceLinkUrlForFQName(project, p.toString().replace(File.separator, "."));
8787
}
88-
return linkUrl.isPresent() ? Renderables.link(path, linkUrl.get()).toMarkdown() : "`"+path+"`";
88+
return linkUrl.isPresent() ? Renderables.link(path, linkUrl.get()).toMarkdown() : Renderables.inlineSnippet(path).toMarkdown();
8989
default:
9090
return path;
9191
}

headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/metadata/util/PropertyDocUtils.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2018 Pivotal, Inc.
2+
* Copyright (c) 2018, 2026 Pivotal, Inc.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -61,7 +61,7 @@ public static Renderable documentJavaElement(SourceLinks sourceLinks, IJavaProje
6161
if (je instanceof IMember) {
6262
IMember member = (IMember) je;
6363
renderableBuilder.add(Renderables.lineBreak());
64-
renderableBuilder.add(Renderables.inlineSnippet(Renderables.text(member.signature())));
64+
renderableBuilder.add(Renderables.inlineSnippet(member.signature()));
6565
IType containingType = je instanceof IType ? (IType) je : ((IMember)je).getDeclaringType();
6666
if (je != null) {
6767
String type = containingType.getFullyQualifiedName();
@@ -70,7 +70,7 @@ public static Renderable documentJavaElement(SourceLinks sourceLinks, IJavaProje
7070
if (url.isPresent()) {
7171
renderableBuilder.add(Renderables.link(type, url.get()));
7272
} else {
73-
renderableBuilder.add(Renderables.inlineSnippet(Renderables.text(type)));
73+
renderableBuilder.add(Renderables.inlineSnippet(type));
7474
}
7575
}
7676
}

0 commit comments

Comments
 (0)