search

FlowingCode / ErrorWindowAddon

Error Window Vaadin Add-on
Apache License 2.0
9 stars 2 forks source link

Error window creation fails if the error text contains html #74

Closed Dudeplayz closed 12 months ago

Dudeplayz commented 1 year ago

An API is returning the following error:

odata.ODataException: ODataError(code=Request-URI Too Long(414), message=<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN""http://www.w3.org/TR/html4/strict.dtd">
<HTML><HEAD><TITLE>Request URL Too Long</TITLE>
<META HTTP-EQUIV="Content-Type" Content="text/html; charset=us-ascii"></HEAD>
<BODY><h2>Request URL Too Long</h2>
<hr><p>HTTP Error 414. The request URL is too long.</p>
</BODY></HTML>

Which results in:

Caused by: java.lang.IllegalArgumentException: HTML must contain exactly one top level element (ignoring text nodes). Found 2 elements with the tag names span, html
    at com.vaadin.flow.component.Html.setOuterHtml(Html.java:145) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.component.Html.<init>(Html.java:113) ~[flow-server-23.3.17.jar:23.3.17]
    at com.flowingcode.vaadin.addons.errorwindow.ErrorWindow.createErrorLabel(ErrorWindow.java:259) ~[error-window-vaadin-3.5.1.jar:3.5.1]
    at com.flowingcode.vaadin.addons.errorwindow.ErrorWindow.createMainLayout(ErrorWindow.java:165) ~[error-window-vaadin-3.5.1.jar:3.5.1]
    at com.flowingcode.vaadin.addons.errorwindow.ErrorWindow.initWindow(ErrorWindow.java:145) ~[error-window-vaadin-3.5.1.jar:3.5.1]
    at com.flowingcode.vaadin.addons.errorwindow.ErrorWindow.lambda$new$26ff2c46$1(ErrorWindow.java:107) ~[error-window-vaadin-3.5.1.jar:3.5.1]
    at com.vaadin.flow.component.ComponentEventBus.fireEventForListener(ComponentEventBus.java:233) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.component.ComponentEventBus.fireEvent(ComponentEventBus.java:222) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.component.Component.fireEvent(Component.java:393) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.component.ComponentUtil.fireEvent(ComponentUtil.java:416) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.component.ComponentUtil.onComponentAttach(ComponentUtil.java:233) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.internal.nodefeature.ComponentMapping.lambda$onAttach$0(ComponentMapping.java:104) ~[flow-server-23.3.17.jar:23.3.17]
    at java.util.Optional.ifPresent(Optional.java:178) ~[?:?]
    at com.vaadin.flow.internal.nodefeature.ComponentMapping.onAttach(ComponentMapping.java:103) ~[flow-server-23.3.17.jar:23.3.17]
    at com.vaadin.flow.internal.StateNode.lambda$fireAttachListeners$8(StateNode.java:858) ~[flow-server-23.3.17.jar:23.3.17]

I don't understand why Span isn't used directly here: https://github.com/FlowingCode/ErrorWindowAddon/blob/26de67daea91d01b9f41b570fd5c701d7a74b98c/src/main/java/com/flowingcode/vaadin/addons/errorwindow/ErrorWindow.java#L313-L315

javier-godoy commented 12 months ago

That code traces back to the initial implementation https://github.com/FlowingCode/ErrorWindowAddon/blob/4334fca3911faecb7e948aa239daafe9127521d5/src/main/java/com/flowingcode/vaadin/addons/errorwindow/ErrorWindow.java#L183

I guess the idea was to allow HTML formatted messages, which in retrospective is a bad idea because it opens a way to script-injection attacks (at least, it is be possible to write a <script> tag in the exception message, and have it executed, which is dangerous because the exception message is not necessarily sanitized). I think we have to change the behavior to interpret the message in text-only format, which would be a (well deserved) breaking change.

As a workaround, you can set your own ErrorWindowFactory implementation by calling ErrorManager.setErrorWindowFactory() from a VaadinServiceInitListener (link to documentation). Then you can copy ErrorWindow and replace that line.

javier-godoy commented 12 months ago

Thank you for bringing this to our attention. The fix has been merged into version 4.0.0-SNAPSHOT, which is available from our snapshot repository (https://maven.flowingcode.com/snapshots). It will be released in the coming days.

Dudeplayz commented 12 months ago

Thanks for the fast fix and good job 👍

javier-godoy commented 12 months ago

Hello. Version 4.0.0 is already available from maven central repository. https://repo1.maven.org/maven2/com/flowingcode/vaadin/addons/error-window-vaadin/4.0.0