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

Panic in template rendering causes dumping of panic in to response #14525

Closed
2 of 6 tasks
CirnoT opened this issue Jan 30, 2021 · 4 comments · Fixed by #14646
Closed
2 of 6 tasks

Panic in template rendering causes dumping of panic in to response #14525

CirnoT opened this issue Jan 30, 2021 · 4 comments · Fixed by #14646
Labels
reviewed/wontfix The problem described in this issue/fixed in this pull request is not a problem we will fix type/bug

Comments

@CirnoT
Copy link
Contributor

CirnoT commented Jan 30, 2021

  • Gitea version (or commit ref): 1.14.0+dev-642-g0e0424c8e
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • Log gist:
2021/01/30 11:16:48 ...els/issue_comment.go:382:LoadLabel() [W] Commit 922 cannot load label 0
2021/01/30 11:16:48 ...els/issue_comment.go:382:LoadLabel() [W] Commit 958 cannot load label 0
2021/01/30 11:16:48 ...els/issue_comment.go:382:LoadLabel() [W] Commit 961 cannot load label 0
2021/01/30 11:16:48 ...s/context/context.go:191:HTML() [E] Render failed: template: repo/issue/view_content/comments:188:51: executing "repo/issue/view_content/comments" at <RenderLabels .AddedLabels>: error calling RenderLabels: runtime error: invalid memory address or nil pointer dereference
2021/01/30 11:16:48 ...c/net/http/server.go:3095:logf() [I] http: superfluous response.WriteHeader call from code.gitea.io/gitea/modules/context.(*Response).WriteHeader (response.go:64)

Description

Template render errors cause HTML content of Internal Server Error page being displayed as text/plain along with full panic message.

@CirnoT CirnoT changed the title Unable to view issues with removed labels superfluous response.WriteHeader call from code.gitea.io Jan 30, 2021
@CirnoT
Copy link
Contributor Author

CirnoT commented Jan 30, 2021

Related to #14466

@lunny lunny added the type/bug label Jan 30, 2021
@zeripath
Copy link
Contributor

zeripath commented Feb 9, 2021

This will be due to the panic writing a header and the original template renderer writing a header.

Unfortunately I don't think there is likely to be a solution here. You have to write the header before you start writing the template but if there is a panic in the template then the panic handler will attempt to write the header again.

So I think we're just gonna have to accept this one as a wontfix

@zeripath zeripath added the reviewed/wontfix The problem described in this issue/fixed in this pull request is not a problem we will fix label Feb 9, 2021
@zeripath zeripath closed this as completed Feb 9, 2021
@CirnoT
Copy link
Contributor Author

CirnoT commented Feb 10, 2021

Are you sure that is correct? Do we really want that if panic occurs during template rendering, the error page's HTML content is dumped as text/plain (so not rendered as HTML)? That seems kind of a weird. Also I really don't think panic message should be exposed to user in production mode.

@zeripath
Copy link
Contributor

@CirnoT your original message was about the writeheader log message - not about it being dumped as plain text. WriteHeader will always have to happen.

I'll reopen. It's really really helpful if the issue is clear from the start as to what the real problem is.

@zeripath zeripath reopened this Feb 11, 2021
@zeripath zeripath changed the title superfluous response.WriteHeader call from code.gitea.io Panic in template rendering causes dumping of panic in to response Feb 11, 2021
zeripath added a commit to zeripath/gitea that referenced this issue Feb 11, 2021
When there is a panic during template rendering unrolled/render
will automatically render the error. This leads to the
panic being displayed in the page and not a 500 page

Fix go-gitea#14467
Fix go-gitea#14525

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit that referenced this issue Feb 13, 2021
When there is a panic during template rendering unrolled/render
will automatically render the error. This leads to the
panic being displayed in the page and not a 500 page

Fix #14467
Fix #14525

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewed/wontfix The problem described in this issue/fixed in this pull request is not a problem we will fix type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants