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

Fix bug of link query order on markdown render #14156

Merged
merged 4 commits into from
Dec 28, 2020

Conversation

lunny
Copy link
Member

@lunny lunny commented Dec 27, 2020

Should fix #14090

We temporarily use a replace on go.mod before microcosm-cc/bluemonday#105 merged.

@lunny lunny added type/bug backport/v1.13 pr/wip This PR is not ready for review labels Dec 27, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Dec 27, 2020
@lunny lunny removed the pr/wip This PR is not ready for review label Dec 27, 2020
@lunny lunny added this to the 1.14.0 milestone Dec 27, 2020
@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 27, 2020
@6543
Copy link
Member

6543 commented Dec 27, 2020

@lunny need to change tests:

--- FAIL: TestRender_links (0.02s)
    html_test.go:89: 
        	Error Trace:	html_test.go:89
        	            				html_test.go:125
        	Error:      	Not equal: 
        	            	expected: "<p><a href=\"https://github.com/go-gitea/gitea/?p=aaa%2Fbbb.html#ccc-ddd\" rel=\"nofollow\">https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd</a></p>"
        	            	actual  : "<p><a href=\"https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd\" rel=\"nofollow\">https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd</a></p>"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-<p><a href="https://github.com/go-gitea/gitea/?p=aaa%2Fbbb.html#ccc-ddd" rel="nofollow">https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd</a></p>
        	            	+<p><a href="https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd" rel="nofollow">https://github.com/go-gitea/gitea/?p=aaa/bbb.html#ccc-ddd</a></p>
        	Test:       	TestRender_links
    html_test.go:89: 
        	Error Trace:	html_test.go:89
        	            				html_test.go:143
        	Error:      	Not equal: 
        	            	expected: "<p><a href=\"magnet:?dn=download&xt=urn%3Abtih%3A5dee65101db281ac9c46344cd6b175cdcadabcde\" rel=\"nofollow\">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>"
        	            	actual  : "<p><a href=\"magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&dn=download\" rel=\"nofollow\">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-<p><a href="magnet:?dn=download&xt=urn%3Abtih%3A5dee65101db281ac9c46344cd6b175cdcadabcde" rel="nofollow">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>
        	            	+<p><a href="magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&dn=download" rel="nofollow">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>
        	Test:       	TestRender_links
FAIL

@zeripath
Copy link
Contributor

no - don't change the tests - fix the query parameter it's not escaping properly.

@6543 6543 added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Dec 27, 2020
@lunny
Copy link
Member Author

lunny commented Dec 27, 2020

The first test should be right and I changed the bluemonday codes.

I also changed the second test on this PR because it should be a wrong test.

html_test.go:143
        	Error:      	Not equal: 
        	            	expected: "<p><a href=\"magnet:?dn=download&xt=urn%3Abtih%3A5dee65101db281ac9c46344cd6b175cdcadabcde\" rel=\"nofollow\">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>"
        	            	actual  : "<p><a href=\"magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&dn=download\" rel=\"nofollow\">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-<p><a href="magnet:?dn=download&xt=urn%3Abtih%3A5dee65101db281ac9c46344cd6b175cdcadabcde" rel="nofollow">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>
        	            	+<p><a href="magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&dn=download" rel="nofollow">magnet:?xt=urn:btih:5dee65101db281ac9c46344cd6b175cdcadabcde&amp;dn=download</a></p>
        	Test:       	TestRender_links

It's obvious, the query order changed but should not.

@codecov-io
Copy link

Codecov Report

Merging #14156 (e67dd5c) into master (cf9d471) will decrease coverage by 0.33%.
The diff coverage is 3.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14156      +/-   ##
==========================================
- Coverage   42.36%   42.02%   -0.34%     
==========================================
  Files         728      733       +5     
  Lines       78015    78666     +651     
==========================================
+ Hits        33049    33059      +10     
- Misses      39540    40179     +639     
- Partials     5426     5428       +2     
Impacted Files Coverage Δ
cmd/dump_repo.go 0.00% <0.00%> (ø)
cmd/restore_repo.go 0.00% <0.00%> (ø)
models/admin.go 60.31% <0.00%> (-8.78%) ⬇️
models/task.go 34.95% <ø> (+0.04%) ⬆️
modules/migrations/base/downloader.go 17.51% <ø> (ø)
modules/migrations/base/pullrequest.go 0.00% <ø> (ø)
modules/migrations/dump.go 0.00% <0.00%> (ø)
modules/migrations/error.go 100.00% <ø> (ø)
modules/migrations/git.go 46.42% <ø> (+3.09%) ⬆️
modules/migrations/gitea_downloader.go 0.91% <0.00%> (+<0.01%) ⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50a2dd5...e67dd5c. Read the comment docs.

@6543 6543 removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Dec 27, 2020
@lunny lunny merged commit 11555d8 into go-gitea:master Dec 28, 2020
@lunny lunny deleted the lunny/fix_urls branch December 28, 2020 16:28
lunny added a commit to lunny/gitea that referenced this pull request Dec 28, 2020
* Fix bug of link query order on markdown render

* Fix bluemonday bug and fix one wrong test

Co-authored-by: 6543 <6543@obermui.de>
techknowlogick pushed a commit that referenced this pull request Dec 28, 2020
* Fix bug of link query order on markdown render

* Fix bluemonday bug and fix one wrong test

Co-authored-by: 6543 <6543@obermui.de>

Co-authored-by: 6543 <6543@obermui.de>
@6543 6543 added the backport/done All backports for this PR have been created label Jan 26, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
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. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

urls in markdown are broken - arguments are sorted
6 participants