Skip to content

Commit 524156e

Browse files
authored
fix: invalid link parser safety (#367)
1 parent a7f65cd commit 524156e

22 files changed

Lines changed: 142 additions & 81 deletions

File tree

common/src/main/java/com/klikli_dev/modonomicon/api/ModonomiconConstants.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public static class Gui {
6868
public static final String HOVER_BOOK_LINK = PREFIX + "hover.book_link";
6969
public static final String HOVER_BOOK_LINK_ERROR = PREFIX + "hover.book_link.error";
7070
public static final String HOVER_BOOK_LINK_LOCKED = PREFIX + "hover.book_link_locked";
71+
public static final String HOVER_LINK_ERROR = PREFIX + "hover.link.error";
7172
public static final String HOVER_BOOK_ENTRY_LINK_LOCKED_INFO = PREFIX + "hover.book_entry_link_locked_info";
7273
public static final String HOVER_BOOK_ENTRY_LINK_LOCKED_INFO_HINT = PREFIX + "hover.book_entry_link_locked_info.hint";
7374
public static final String HOVER_BOOK_PAGE_LINK_LOCKED_INFO = PREFIX + "hover.book_page_link_locked_info";

common/src/main/java/com/klikli_dev/modonomicon/book/BookTextHolder.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88

99
import com.mojang.datafixers.util.Either;
1010
import com.mojang.serialization.Codec;
11+
import net.minecraft.client.resources.language.I18n;
1112
import net.minecraft.network.RegistryFriendlyByteBuf;
1213
import net.minecraft.network.codec.ByteBufCodecs;
1314
import net.minecraft.network.codec.StreamCodec;
14-
import net.minecraft.client.resources.language.I18n;
1515
import net.minecraft.network.chat.Component;
1616
import net.minecraft.network.chat.ComponentSerialization;
1717
import net.minecraft.network.chat.contents.TranslatableContents;
@@ -21,7 +21,7 @@ public class BookTextHolder {
2121

2222
public static final BookTextHolder EMPTY = new BookTextHolder("");
2323
public static final Codec<BookTextHolder> CODEC = Codec.either(Codec.STRING, ComponentSerialization.CODEC)
24-
.xmap(value -> value.map(BookTextHolder::new, BookTextHolder::new), holder -> holder.hasComponent() ? Either.right(holder.component) : Either.left(holder.string));
24+
.xmap(value -> value.map(BookTextHolder::new, BookTextHolder::new), BookTextHolder::toSerializableValue);
2525
public static final StreamCodec<RegistryFriendlyByteBuf, BookTextHolder> STREAM_CODEC = ByteBufCodecs.fromCodecWithRegistries(CODEC);
2626

2727
private Component component;
@@ -73,6 +73,10 @@ public void toNetwork(RegistryFriendlyByteBuf buffer) {
7373
STREAM_CODEC.encode(buffer, this);
7474
}
7575

76+
protected Either<String, Component> toSerializableValue() {
77+
return this.hasComponent() ? Either.right(this.component) : Either.left(this.string);
78+
}
79+
7680
@Override
7781
public boolean equals(Object o) {
7882
if (this == o) {

common/src/main/java/com/klikli_dev/modonomicon/book/RenderedBookTextHolder.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package com.klikli_dev.modonomicon.book;
88

9+
import com.mojang.datafixers.util.Either;
910
import net.minecraft.network.RegistryFriendlyByteBuf;
1011
import net.minecraft.network.chat.Component;
1112
import net.minecraft.network.chat.MutableComponent;
@@ -18,6 +19,12 @@ public class RenderedBookTextHolder extends BookTextHolder {
1819
private final List<MutableComponent> renderedText;
1920

2021
public RenderedBookTextHolder(BookTextHolder original, List<MutableComponent> renderedText) {
22+
if (original == null) {
23+
throw new IllegalArgumentException("original cannot be null");
24+
}
25+
if (renderedText == null) {
26+
throw new IllegalArgumentException("renderedText cannot be null");
27+
}
2128
this.original = original;
2229
this.renderedText = renderedText;
2330
}
@@ -50,4 +57,9 @@ public boolean hasComponent() {
5057
public void toNetwork(RegistryFriendlyByteBuf buffer) {
5158
this.original.toNetwork(buffer);
5259
}
60+
61+
@Override
62+
protected Either<String, Component> toSerializableValue() {
63+
return this.original.toSerializableValue();
64+
}
5365
}

common/src/main/java/com/klikli_dev/modonomicon/book/error/BookErrorHolder.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,12 @@ public void addError(BookErrorInfo error) {
2020
public List<BookErrorInfo> getErrors() {
2121
return this.errors;
2222
}
23+
24+
public boolean hasBlockingErrors() {
25+
return this.errors.stream().anyMatch(BookErrorInfo::blocksOpening);
26+
}
27+
28+
public BookErrorInfo getFirstBlockingError() {
29+
return this.errors.stream().filter(BookErrorInfo::blocksOpening).findFirst().orElse(null);
30+
}
2331
}

common/src/main/java/com/klikli_dev/modonomicon/book/error/BookErrorInfo.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,21 @@ public class BookErrorInfo {
1010
private final String errorMessage;
1111
private final Exception exception;
1212
private final String context;
13+
private final boolean blocksOpening;
1314

1415
public BookErrorInfo(String errorMessage, Exception exception, String context) {
16+
this(errorMessage, exception, context, true);
17+
}
18+
19+
public BookErrorInfo(String errorMessage, Exception exception, String context, boolean blocksOpening) {
1520
this.errorMessage = errorMessage;
1621
this.exception = exception;
1722
this.context = context;
23+
this.blocksOpening = blocksOpening;
24+
}
25+
26+
public boolean blocksOpening() {
27+
return this.blocksOpening;
1828
}
1929

2030
@Override
@@ -26,6 +36,7 @@ public String toString() {
2636
"\nerrorMessage='" + errorMessage + "'" +
2737
", \ncontext='" + context + "'" +
2838
", \nexception='" + exception + "'" +
39+
", \nblocksOpening='" + this.blocksOpening + "'" +
2940
"\n}";
3041
}
3142
}

common/src/main/java/com/klikli_dev/modonomicon/book/error/BookErrorManager.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ public boolean hasErrors(Identifier book) {
5252
return holder != null && !holder.getErrors().isEmpty();
5353
}
5454

55+
public boolean hasBlockingErrors(Identifier book) {
56+
var holder = this.booksErrors.get(book);
57+
return holder != null && holder.hasBlockingErrors();
58+
}
59+
5560
public void error(String message) {
5661
this.error(new BookErrorInfo(message, null, this.currentContext));
5762
}
@@ -60,6 +65,10 @@ public void error(String message, Exception exception) {
6065
this.error(new BookErrorInfo(message, exception, this.currentContext));
6166
}
6267

68+
public void error(String message, Exception exception, boolean blocksOpening) {
69+
this.error(new BookErrorInfo(message, exception, this.currentContext, blocksOpening));
70+
}
71+
6372
public void error(BookErrorInfo error) {
6473
this.error(this.currentBookId, error);
6574
}
@@ -72,6 +81,10 @@ public void error(Identifier book, String message, Exception exception) {
7281
this.error(book, new BookErrorInfo(message, exception, this.currentContext));
7382
}
7483

84+
public void error(Identifier book, String message, Exception exception, boolean blocksOpening) {
85+
this.error(book, new BookErrorInfo(message, exception, this.currentContext, blocksOpening));
86+
}
87+
7588
public void error(Identifier book, BookErrorInfo error) {
7689
if (book == null) {
7790
Modonomicon.LOG.error("BookErrorManager.error() called with null book id with error: {}", error);

common/src/main/java/com/klikli_dev/modonomicon/client/gui/BookGuiManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static BookGuiManager get() {
7272
}
7373

7474
protected boolean showErrorScreen(Identifier bookId) {
75-
if (BookErrorManager.get().hasErrors(bookId)) {
75+
if (BookErrorManager.get().hasBlockingErrors(bookId)) {
7676
var book = BookDataManager.get().getBook(bookId);
7777
Minecraft.getInstance().setScreen(new BookErrorScreen(book));
7878
return true;
@@ -425,6 +425,7 @@ public void openEntry(Identifier bookId, @Nullable Identifier categoryId, @Nulla
425425
}
426426

427427
if (this.showErrorScreen(bookId)) {
428+
return;
428429
}
429430

430431
this.keepMousePosition(() -> {

common/src/main/java/com/klikli_dev/modonomicon/client/gui/book/BookErrorScreen.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public void renderBookBackground(GuiGraphicsExtractor guiGraphics) {
3838

3939
public void renderError(GuiGraphicsExtractor guiGraphics, Component text, int x, int y, int width) {
4040
for (FormattedCharSequence formattedcharsequence : this.font.split(text, width)) {
41-
guiGraphics.text(this.font, formattedcharsequence, x, y, 1, false);
41+
guiGraphics.text(this.font, formattedcharsequence, x, y, 0xFF000000, false);
4242
y += this.font.lineHeight;
4343
}
4444
}
@@ -49,7 +49,10 @@ public void prepareError() {
4949
this.errorText = Component.translatable(Gui.NO_ERRORS_FOUND);
5050
Modonomicon.LOG.warn("No errors found for book {}, but error screen was opened!", this.book.getId());
5151
} else {
52-
var firstError = errorHolder.getErrors().get(0);
52+
var firstError = errorHolder.getFirstBlockingError();
53+
if (firstError == null) {
54+
firstError = errorHolder.getErrors().get(0);
55+
}
5356

5457
var errorString = firstError.toString();
5558
if (errorHolder.getErrors().size() > 1) {

common/src/main/java/com/klikli_dev/modonomicon/client/gui/book/markdown/BookLinkRenderer.java

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
package com.klikli_dev.modonomicon.client.gui.book.markdown;
88

9-
import com.klikli_dev.modonomicon.Modonomicon;
109
import com.klikli_dev.modonomicon.api.ModonomiconConstants.I18n.Gui;
1110
import com.klikli_dev.modonomicon.book.BookLink;
1211
import com.klikli_dev.modonomicon.book.error.BookErrorManager;
@@ -31,7 +30,6 @@ public boolean visit(Link link, Consumer<Node> visitChildren, ComponentNodeRende
3130
link.getDestination(),
3231
BookErrorManager.get().getContextHelper()
3332
);
34-
3533
try {
3634
var bookLink = BookLink.from(context.getBook(), link.getDestination());
3735
var book = BookDataManager.get().getBook(bookLink.bookId);
@@ -63,34 +61,10 @@ public boolean visit(Link link, Consumer<Node> visitChildren, ComponentNodeRende
6361
.withClickEvent(null)
6462
.withHoverEvent(null)
6563
);
66-
} catch (Exception e) {
67-
if (context.getBook().allowOpenBooksWithInvalidLinks()) {
68-
Modonomicon.LOG.error("Failed to parse book link. allowOpenBooksWithInvalidLinks = true, so book parsing continues. Original error:", e);
69-
70-
//Render error message as tooltip in red
71-
var hoverComponent = Component.translatable(Gui.HOVER_BOOK_LINK_ERROR, link.getDestination()).withStyle(ChatFormatting.RED);
72-
73-
//Render link in red
74-
context.setCurrentStyle(context.getCurrentStyle()
75-
.withColor(ChatFormatting.RED)
76-
.withHoverEvent(new HoverEvent.ShowText(hoverComponent))
77-
);
78-
79-
visitChildren.accept(link);
80-
81-
//links are not style instructions, so we reset to our previous color.
82-
context.setCurrentStyle(context.getCurrentStyle()
83-
.withColor(currentColor)
84-
.withClickEvent(null)
85-
.withHoverEvent(null)
86-
);
87-
} else {
88-
throw e;
89-
}
64+
return true;
65+
} finally {
66+
BookErrorManager.get().setContext(null);
9067
}
91-
92-
BookErrorManager.get().setContext(null);
93-
return true;
9468
}
9569
return false;
9670

common/src/main/java/com/klikli_dev/modonomicon/client/gui/book/markdown/CoreComponentNodeRenderer.java

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@
66

77
package com.klikli_dev.modonomicon.client.gui.book.markdown;
88

9+
import com.klikli_dev.modonomicon.Modonomicon;
910
import com.klikli_dev.modonomicon.api.ModonomiconConstants.I18n.Gui;
11+
import com.klikli_dev.modonomicon.book.error.BookErrorManager;
1012
import com.klikli_dev.modonomicon.client.gui.book.markdown.internal.renderer.BulletListHolder;
1113
import com.klikli_dev.modonomicon.client.gui.book.markdown.internal.renderer.OrderedListHolder;
1214
import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;
15+
import net.minecraft.ChatFormatting;
1316
import net.minecraft.network.chat.ClickEvent;
1417
import net.minecraft.network.chat.ClickEvent.Action;
1518
import net.minecraft.network.chat.Component;
@@ -101,25 +104,52 @@ public void visit(HardLineBreak hardLineBreak) {
101104

102105
@Override
103106
public void visit(Link link) {
104-
for (var renderer : this.context.getLinkRenderers()) {
105-
if (renderer.visit(link, this::visitChildren, this.context)) {
106-
return;
107+
try {
108+
for (var renderer : this.context.getLinkRenderers()) {
109+
if (renderer.visit(link, this::visitChildren, this.context)) {
110+
return;
111+
}
107112
}
113+
114+
//if no link renderer, we just do a http link
115+
var currentColor = this.context.getCurrentStyle().getColor();
116+
var hoverComponent = Component.translatable(Gui.HOVER_HTTP_LINK, link.getDestination());
117+
//if we have a color we use it, otherwise we use link default.
118+
this.context.setCurrentStyle(this.context.getCurrentStyle()
119+
.withColor(currentColor == null ? this.context.getLinkColor() : currentColor)
120+
.withClickEvent(new ClickEvent.OpenUrl(URI.create(link.getDestination())))
121+
.withHoverEvent(new HoverEvent.ShowText(hoverComponent))
122+
);
123+
124+
this.visitChildren(link);
125+
126+
//at the end of the link we reset to our previous color.
127+
this.context.setCurrentStyle(this.context.getCurrentStyle()
128+
.withColor(currentColor)
129+
.withClickEvent(null)
130+
.withHoverEvent(null)
131+
);
132+
} catch (Exception e) {
133+
this.renderBrokenLink(link, e);
108134
}
135+
}
136+
137+
private void renderBrokenLink(Link link, Exception exception) {
138+
var blocksOpening = !this.context.getBook().allowOpenBooksWithInvalidLinks();
139+
BookErrorManager.get().error("Failed to parse markdown link '" + link.getDestination() + "'", exception, blocksOpening);
140+
Modonomicon.LOG.error("Failed to parse markdown link. allowOpenBooksWithInvalidLinks = {}, blocksOpening = {}. Original error:",
141+
this.context.getBook().allowOpenBooksWithInvalidLinks(), blocksOpening, exception);
109142

110-
//if no link renderer, we just do a http link
111143
var currentColor = this.context.getCurrentStyle().getColor();
112-
var hoverComponent = Component.translatable(Gui.HOVER_HTTP_LINK, link.getDestination());
113-
//if we have a color we use it, otherwise we use link default.
144+
var hoverComponent = Component.translatable(Gui.HOVER_LINK_ERROR, link.getDestination()).withStyle(ChatFormatting.RED);
114145
this.context.setCurrentStyle(this.context.getCurrentStyle()
115-
.withColor(currentColor == null ? this.context.getLinkColor() : currentColor)
116-
.withClickEvent(new ClickEvent.OpenUrl(URI.create(link.getDestination())))
146+
.withColor(ChatFormatting.RED)
147+
.withClickEvent(null)
117148
.withHoverEvent(new HoverEvent.ShowText(hoverComponent))
118149
);
119150

120151
this.visitChildren(link);
121152

122-
//at the end of the link we reset to our previous color.
123153
this.context.setCurrentStyle(this.context.getCurrentStyle()
124154
.withColor(currentColor)
125155
.withClickEvent(null)

0 commit comments

Comments
 (0)