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: index out of range when viewing org file in repository #4982

Closed
2 of 7 tasks
howdoicomputer opened this issue Sep 26, 2018 · 8 comments · Fixed by #5903
Closed
2 of 7 tasks

PANIC: index out of range when viewing org file in repository #4982

howdoicomputer opened this issue Sep 26, 2018 · 8 comments · Fixed by #5903
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@howdoicomputer
Copy link

howdoicomputer commented Sep 26, 2018

Description

I checked in an org file to a private repository. I'm able to pull the file and view the diff but rendering it in the web view produces a blank page and stack trace.

@howdoicomputer
Copy link
Author

It looks like Gitea fails to render if goorgeous fails to parse. I pushed a simpler test.org file and it rendered fine.

@howdoicomputer
Copy link
Author

howdoicomputer commented Sep 26, 2018

This is an upstream issue: curiouslychase/_goorgeous#82

Goorgeous cannot parse org files with empty headerlines.

@lafriks lafriks added type/bug status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR labels Sep 26, 2018
@yasuokav yasuokav mentioned this issue Jan 30, 2019
7 tasks
@zeripath zeripath added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jan 30, 2019
@zeripath
Copy link
Contributor

Ok, although the original cause of this bug is in an upstream library - now that we know about it we should recover from panics within that library and just render the text with an error.

See https://blog.golang.org/defer-panic-and-recover

We should also have a root handler for Macaron that does a similar recover and returns a gitea 500. Probably with two different levels of information depending on the production mode. (I.e. report stack trace or not)

@adelowo
Copy link
Member

adelowo commented Jan 30, 2019

@zeripath There is already a handler for recovery. I believe the application should keep on
running. I have run into a few waves of panic too

m.Use(macaron.Recovery())

https://github.com/go-macaron/docs/blob/master/en-US/middlewares/core_services.md

@zeripath
Copy link
Contributor

Ah but that is not good enough - the user gets a blank page.

@igor9292
Copy link

igor9292 commented Jan 30, 2019

colleagues, and you can make the file appear just text,
file was displayed without formatting?

zeripath added a commit to zeripath/gitea that referenced this issue Jan 30, 2019
This PR protects against the panic referred to in chaseadmsio/goorgeous#82
by recovering from the panic and just returning the raw bytes if
there is an error.

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added in progress and removed status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR labels Jan 30, 2019
@zeripath
Copy link
Contributor

I'm removing the status/blocked label. Yes we can't prevent goorgeous from having a panic but we can damned well stop that propogating - and we should. I've put a PR up to recover from the panic and just return the original byte array as plain text. This is the simplest thing to do but is perhaps a little unfriendly - another solution might be to return "Unable to render" instead.

@howdoicomputer
Copy link
Author

howdoicomputer commented Jan 30, 2019

There is also this open issue that suggests replacing goorgeous with another library.

#5710

techknowlogick pushed a commit that referenced this issue Jan 30, 2019
This PR protects against the panic referred to in chaseadmsio/goorgeous#82
by recovering from the panic and just returning the raw bytes if
there is an error.

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue Feb 16, 2019
…tea#5903)

This PR protects against the panic referred to in chaseadmsio/goorgeous#82
by recovering from the panic and just returning the raw bytes if
there is an error.

Signed-off-by: Andrew Thornton <[email protected]>
techknowlogick pushed a commit that referenced this issue Feb 16, 2019
This PR protects against the panic referred to in chaseadmsio/goorgeous#82
by recovering from the panic and just returning the raw bytes if
there is an error.

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants