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

Ensure that plain files are rendered correctly even when containing ambiguous characters #22017

Merged

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Dec 3, 2022

As recognised in #21841 the rendering of plain text files is somewhat incorrect when there are ambiguous characters as the html code is double escaped. In fact there are several more problems here.

We have a residual isRenderedHTML which is actually simply escaping the file - not rendering it. This is badly named and gives the wrong impression.

There is also unusual behaviour whether the file is called a Readme or not and there is no way to get to the source code if the file is called README.

In reality what should happen is different depending on whether the file is being rendered a README at the bottom of the directory view or not.

  1. If it is rendered as a README on a directory - it should simply be escaped and rendered as <pre> text.
  2. If it is rendered as a file then it should be rendered as source code.

This PR therefore does:

  1. Rename IsRenderedHTML to IsPlainText
  2. Readme files rendered at the bottom of the directory are rendered without line numbers
  3. Otherwise plain text files are rendered as source code.

Replace #21841

Signed-off-by: Andrew Thornton art27@cantab.net

…mbiguous characters

As recognised in go-gitea#21841 the rendering of plain text files is somewhat
incorrect when there are ambiguous characters as the html code is double
escaped. In fact there are several more problems here.

We have a residual isRenderedHTML which is actually simply escaping the
file - not rendering it. This is badly named and gives the wrong
impression.

There is also unusual behaviour whether the file is called a Readme or
not and there is no way to get to the source code if the file is called
README.

In reality what should happen is different depending on whether the
file is being rendered a README at the bottom of the directory view or
not.

1. If it is rendered as a README on a directory - it should simply be
   escaped and rendered as `<pre>` text.
2. If it is rendered as a file then it should be rendered as source
   code.

This PR therefore does:
1. Rename IsRenderedHTML to IsRenderedPlainText
2. Readme files rendered at the bottom of the directory are rendered without line numbers
3. Otherwise plain text files are rendered as source code.

Replace go-gitea#21841

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/bug outdated/backport/v1.18 This PR should be backported to Gitea 1.18 labels Dec 3, 2022
@zeripath zeripath added this to the 1.18.0 milestone Dec 3, 2022
@zeripath zeripath modified the milestones: 1.18.0, 1.19.0 Dec 3, 2022
routers/web/repo/view.go Outdated Show resolved Hide resolved
modules/charset/escape.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 3, 2022
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 5, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 16, 2022
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 17, 2022
@lafriks lafriks removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 17, 2022
@lafriks
Copy link
Member

lafriks commented Dec 17, 2022

🚀

@lafriks lafriks merged commit 6e22605 into go-gitea:main Dec 17, 2022
@lunny
Copy link
Member

lunny commented Dec 18, 2022

Please send backport

@zeripath zeripath deleted the replace-21841-correctly-render-plain-text-files branch December 18, 2022 12:17
zeripath added a commit to zeripath/gitea that referenced this pull request Dec 18, 2022
…mbiguous characters (go-gitea#22017)

Backport go-gitea#22017

As recognised in go-gitea#21841 the rendering of plain text files is somewhat
incorrect when there are ambiguous characters as the html code is double
escaped. In fact there are several more problems here.

We have a residual isRenderedHTML which is actually simply escaping the
file - not rendering it. This is badly named and gives the wrong
impression.

There is also unusual behaviour whether the file is called a Readme or
not and there is no way to get to the source code if the file is called
README.

In reality what should happen is different depending on whether the file
is being rendered a README at the bottom of the directory view or not.

1. If it is rendered as a README on a directory - it should simply be
escaped and rendered as `<pre>` text.
2. If it is rendered as a file then it should be rendered as source
code.

This PR therefore does:
1. Rename IsRenderedHTML to IsPlainText
2. Readme files rendered at the bottom of the directory are rendered
without line numbers
3. Otherwise plain text files are rendered as source code.

Replace go-gitea#21841

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@zeripath zeripath added the backport/done All backports for this PR have been created label Dec 18, 2022
lunny added a commit that referenced this pull request Dec 19, 2022
…mbiguous characters (#22017) (#22160)

Backport #22017

As recognised in #21841 the rendering of plain text files is somewhat
incorrect when there are ambiguous characters as the html code is double
escaped. In fact there are several more problems here.

We have a residual isRenderedHTML which is actually simply escaping the
file - not rendering it. This is badly named and gives the wrong
impression.

There is also unusual behaviour whether the file is called a Readme or
not and there is no way to get to the source code if the file is called
README.

In reality what should happen is different depending on whether the file
is being rendered a README at the bottom of the directory view or not.

1. If it is rendered as a README on a directory - it should simply be
escaped and rendered as `<pre>` text.
2. If it is rendered as a file then it should be rendered as source
code.

This PR therefore does:
1. Rename IsRenderedHTML to IsPlainText
2. Readme files rendered at the bottom of the directory are rendered
without line numbers
3. Otherwise plain text files are rendered as source code.

Replace #21841

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.18 This PR should be backported to Gitea 1.18 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants