Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error page has HTML error when writePoweredBy is enabled. #6520

Closed
gencarnacao opened this issue Jul 16, 2021 · 6 comments · Fixed by #6525 or #6526
Closed

Error page has HTML error when writePoweredBy is enabled. #6520

gencarnacao opened this issue Jul 16, 2021 · 6 comments · Fixed by #6525 or #6526
Labels
Bug For general bugs on Jetty side

Comments

@gencarnacao
Copy link

gencarnacao commented Jul 16, 2021

Jetty version(s)
At least since 9.4.x

Description
The footer of the page, containing the "Powered By" line has an incorrect closing html tag. This causes issues when trying to parse the file automatically although visually in browsers this is undetectable.

How to reproduce?
With the Powered By footer enabled, trigger any error page (easiest is 404). I detected this on a running Jenkins installation while trying to access the jobs api of a non-existent job.

https://github.com/eclipse/jetty.project/blob/9d6db9e0986b352e54c7a03a3a1564179b69e10d/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java#L400

I can avoid the problem by setting the 'Accept' header to 'application/json' which then returns a correct payload and is parseable.

@gencarnacao gencarnacao added the Bug For general bugs on Jetty side label Jul 16, 2021
@sbordet
Copy link
Contributor

sbordet commented Jul 16, 2021

@gencarnacao as the fix is trivial, would you be interested in issuing a PR to fix it?

@joakime
Copy link
Contributor

joakime commented Jul 16, 2021

We validate that the JSON error output is sane in our ErrorHandlerTest.
Perhaps we should do the same with the HTML (we should be generating XHTML compliant output) and the XML output too.

@gencarnacao
Copy link
Author

gencarnacao commented Jul 16, 2021

@sbordet I tried. There should be a PR pending. Somewhere.

joakime added a commit that referenced this issue Jul 16, 2021
+ Updating tests to ensure that output is xml verified
+ Updating output to use `<hr>` element properly.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime linked a pull request Jul 16, 2021 that will close this issue
@joakime
Copy link
Contributor

joakime commented Jul 16, 2021

Opened proper fix (with testcase to verify output format validitiy) as PR #6525

@gencarnacao
Copy link
Author

gencarnacao commented Jul 16, 2021

Saw the PR, unless I misundertood I think there is a typo.
You modified the opening <hr> tag not the closing <hr/> one which should be </hr>.

@joakime
Copy link
Contributor

joakime commented Jul 16, 2021

The <hr> element (aka the "Horizontal Rule") has no body.

So this is invalid: <hr>body</hr>
but this is not: <hr/>body<hr/>.

While in strict HTML5 you don't have to close the <hr> element, you do have to close it if you want to conform with XHTML.

joakime added a commit that referenced this issue Jul 17, 2021
…alid-xhtml-xml

Issue #6520 - Fixing ErrorHandler output of text/html
joakime added a commit that referenced this issue Jul 17, 2021
+ Updating tests to ensure that output is xml verified
+ Updating output to use `<hr>` element properly.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime linked a pull request Jul 17, 2021 that will close this issue
joakime added a commit that referenced this issue Jul 23, 2021
…valid-xhtml-xml

Issue #6520 - Fixing ErrorHandler output of text/html
@sbordet sbordet added this to To do in Jetty 9.4.44 FROZEN via automation Sep 20, 2021
@sbordet sbordet added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Sep 20, 2021
@sbordet sbordet moved this from To do to Done in Jetty 10.0.7/11.0.7 FROZEN Sep 20, 2021
@sbordet sbordet moved this from To do to Done in Jetty 9.4.44 FROZEN Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
3 participants