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

[Bug Report] Relative links in markdown #18592

Closed
schorsch13 opened this issue Feb 4, 2022 · 41 comments · Fixed by #32612
Closed

[Bug Report] Relative links in markdown #18592

schorsch13 opened this issue Feb 4, 2022 · 41 comments · Fixed by #32612
Labels

Comments

@schorsch13
Copy link
Contributor

Introduction

When using relative links in markdown files you have to use either ./file.md or file.md. When trying to use /file.md you will get redirected to the root of the gitea instance.

Example

https://try.gitea.io/schorsch/relative-links-in-markdown

image

Proposal

Change /file.md from directing to the web root to the project root

Credits

The bug was discovered by the codeberg user ivan-paleo. For further information have a look at the issue at codeberg: https://codeberg.org/Codeberg/Community/issues/252

@zeripath
Copy link
Contributor

zeripath commented Feb 4, 2022

This was fixed at some point and something has caused a regression!!

@zeripath
Copy link
Contributor

zeripath commented Feb 4, 2022

Yes this was fixed in #15088

This is particularly irritating as there was even a test created to prevent this from happening YET AGAIN

@zeripath
Copy link
Contributor

zeripath commented Feb 4, 2022

Let's just go through this again.

The base url for an image link rendered in a file GH is:

/owner/repo/blob/branch/path/to/file/

Relative links may traverse out of the repo i.e. if a file that is in / of owner/repo on the default branch master has a link:

../../../../owner2/repo2

then it would like to /owner2/repo2 directly.

Links that start with a / e.g. /../../../owner2/repo2 do not traverse out of the repo. This is not true!

Essentially a leading / is dropped from the links and then the path is made relative to the branch link


I'll do some more test cases in pathological and properly add tests to gitea to prevent this from happening yet again.


Now in issues and presumably elsewhere the opposite happens:

A

/A

../A

../../A

../../../A
../../../../A
../../../../../A
../../../../../../A

/../A

@zeripath
Copy link
Contributor

zeripath commented Feb 5, 2022

OK I guess we need the following (this is for a gitea mounted on a suburl /gitea):

name input file fileInDir wiki issue
samedir file /gitea/gogits/gogs/src/branch/master/file /gitea/gogits/gogs/src/branch/master/subdir/file /gitea/gogits/gogs/wiki/file /gitea/gogits/gogs/issues/file
childir subdir/file /gitea/gogits/gogs/src/branch/master/subdir/file /gitea/gogits/gogs/src/branch/master/subdir/subdir/file /gitea/gogits/gogs/wiki/subdir/file /gitea/gogits/gogs/issues/subdir/file
/file /file /gitea/gogits/gogs/src/branch/master/file /gitea/gogits/gogs/src/branch/master/file /gitea/file /gitea/file
../file ../file /gitea/gogits/gogs/src/branch/file /gitea/gogits/gogs/src/branch/master/file /gitea/gogits/gogs/file /gitea/gogits/gogs/file
/../file /../file /gitea/gogits/gogs/src/branch/file /gitea/gogits/gogs/src/branch/file /gitea/file /gitea/file
../../file ../../file /gitea/gogits/gogs/src/file /gitea/gogits/gogs/src/branch/file /gitea/gogits/file /gitea/gogits/file
/../../file /../../file /gitea/gogits/gogs/src/file /gitea/gogits/gogs/src/file /gitea/file /gitea/file
../../../file ../../../file /gitea/gogits/gogs/file /gitea/gogits/gogs/src/file /gitea/file /gitea/file
../../../../file ../../../../file /gitea/gogits/file /gitea/gogits/gogs/file /gitea/file /gitea/file
../../../../../file ../../../../../file /gitea/file /gitea/gogits/file /gitea/file /gitea/file
../../../../../../file ../../../../../../file /gitea/file /gitea/file /gitea/file /gitea/file
../../../../../../../file ../../../../../../../file /gitea/file /gitea/file /gitea/file /gitea/file

One slight problem is that this this is still not github compatible - githubs blob urls are:

https://github.com/zeripath/pathological/blob/master/README.md

Whereas the equivalent in Gitea:

https://try.gitea.io/arandomer/pathological/src/branch/master/README.md

So we're already slightly incompatible.

However, the / behaviour would be the least we should do.

@zeripath
Copy link
Contributor

zeripath commented Feb 5, 2022

TestCase to add to modules/markup/markdown/markdown_test.go
func TestRender_RelativeLinks(t *testing.T) {
	setting.AppURL = "https://localhost:3000/gitea/"
	setting.AppSubURL = "/gitea"

	testcases := []struct {
		name              string
		input             string
		expectedFile      string
		expectedFileInDir string
		expectedWiki      string
		expectedIssue     string
	}{
		{
			name:              "samedir",
			input:             "file",
			expectedFile:      "/gitea/gogits/gogs/src/branch/master/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/master/subdir/file",
			expectedWiki:      "/gitea/gogits/gogs/wiki/file",
			expectedIssue:     "/gitea/gogits/gogs/issues/file",
		},
		{
			name:              "childir",
			input:             "subdir/file",
			expectedFile:      "/gitea/gogits/gogs/src/branch/master/subdir/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/master/subdir/subdir/file",
			expectedWiki:      "/gitea/gogits/gogs/wiki/subdir/file",
			expectedIssue:     "/gitea/gogits/gogs/issues/subdir/file",
		},
		{
			name:              "/file",
			input:             "/file",
			expectedFile:      "/gitea/gogits/gogs/src/branch/master/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/master/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../file",
			input:             "../file",
			expectedFile:      "/gitea/gogits/gogs/src/branch/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/master/file",
			expectedWiki:      "/gitea/gogits/gogs/file",
			expectedIssue:     "/gitea/gogits/gogs/file",
		},
		{
			name:              "/../file",
			input:             "/../file",
			expectedFile:      "/gitea/gogits/gogs/src/branch/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../file",
			input:             "../../file",
			expectedFile:      "/gitea/gogits/gogs/src/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/branch/file",
			expectedWiki:      "/gitea/gogits/file",
			expectedIssue:     "/gitea/gogits/file",
		},
		{
			name:              "/../../file",
			input:             "/../../file",
			expectedFile:      "/gitea/gogits/gogs/src/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../../file",
			input:             "../../../file",
			expectedFile:      "/gitea/gogits/gogs/file",
			expectedFileInDir: "/gitea/gogits/gogs/src/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../../../file",
			input:             "../../../../file",
			expectedFile:      "/gitea/gogits/file",
			expectedFileInDir: "/gitea/gogits/gogs/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../../../../file",
			input:             "../../../../../file",
			expectedFile:      "/gitea/file",
			expectedFileInDir: "/gitea/gogits/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../../../../../file",
			input:             "../../../../../../file",
			expectedFile:      "/gitea/file",
			expectedFileInDir: "/gitea/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
		{
			name:              "../../../../../../../file",
			input:             "../../../../../../../file",
			expectedFile:      "/gitea/file",
			expectedFileInDir: "/gitea/file",
			expectedWiki:      "/gitea/file",
			expectedIssue:     "/gitea/file",
		},
	}

	for _, testcase := range testcases {
		t.Run(testcase.name, func(t *testing.T) {
			t.Run("file", func(t *testing.T) {
				buffer, err := RenderString(&markup.RenderContext{
					URLPrefix: setting.AppSubURL + "/" + Repo + "/src/branch/master/",
					Filename:  "test.md",
					Metas: map[string]string{
						"user":     "gogits",
						"repo":     "gogs",
						"repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/",
					},
				}, "["+testcase.input+"]("+testcase.input+")")
				assert.NoError(t, err)
				assert.Equal(t, fmt.Sprintf("<p><a href=\"%s\" rel=\"nofollow\">%s</a></p>", testcase.expectedFile, testcase.input), strings.TrimSpace(buffer), "Incorrect file path")
			})
			t.Run("fileInDir", func(t *testing.T) {
				buffer, err := RenderString(&markup.RenderContext{
					URLPrefix: setting.AppSubURL + "/" + Repo + "/src/branch/master/subdir/",
					Filename:  "test.md",
					Metas: map[string]string{
						"user":     "gogits",
						"repo":     "gogs",
						"repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/",
					},
				}, "["+testcase.input+"]("+testcase.input+")")
				assert.NoError(t, err)
				assert.Equal(t, fmt.Sprintf("<p><a href=\"%s\" rel=\"nofollow\">%s</a></p>", testcase.expectedFileInDir, testcase.input), strings.TrimSpace(buffer), "Incorrect subdir file path")
			})
			t.Run("wiki", func(t *testing.T) {
				buffer, err := RenderString(&markup.RenderContext{
					URLPrefix: setting.AppSubURL + "/" + Repo,
					IsWiki:    true,
					Metas: map[string]string{
						"user":     "gogits",
						"repo":     "gogs",
						"repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/",
					},
				}, "["+testcase.input+"]("+testcase.input+")")
				assert.NoError(t, err)
				assert.Equal(t, fmt.Sprintf("<p><a href=\"%s\" rel=\"nofollow\">%s</a></p>", testcase.expectedWiki, testcase.input), strings.TrimSpace(buffer), "Incorrect wiki path")
			})
			t.Run("comment", func(t *testing.T) {
				buffer, err := RenderString(&markup.RenderContext{
					URLPrefix: setting.AppSubURL + "/" + Repo + "/issues/",
					Metas: map[string]string{
						"user":     "gogits",
						"repo":     "gogs",
						"repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/",
						"mode":     "comment",
					},
				}, "["+testcase.input+"]("+testcase.input+")")
				assert.NoError(t, err)
				assert.Equal(t, fmt.Sprintf("<p><a href=\"%s\" rel=\"nofollow\">%s</a></p>", testcase.expectedIssue, testcase.input), strings.TrimSpace(buffer), "Incorrect issue path")
			})
		})
	}
}

@charles-997
Copy link

Has there been any progress on this bug? This seems like a reasonably important feature-breaking bug!
I was surprised to see that my relative links in existing repositories do not function properly in Gitea.

@xbreaker
Copy link

xbreaker commented Dec 28, 2022

Same problem with using HTML A tag in markdown with relative file name

Steps:

  1. Create markdown file with HTML tag A and link to local file (for example, <a href="readme.md>readme</a>)
  2. Save file, link will lost current path, if you viewing branch link will be /repo/src/branch/file.md instead of /repo/src/branch/main/file.md

Demo: https://try.gitea.io/aybe/markdown-branch/

@lunny
Copy link
Member

lunny commented May 9, 2023

From https://try.gitea.io/aybe/markdown-branch/ , only relative link from a tag is wrong. Maybe it's accepted?

@ell1e
Copy link

ell1e commented Sep 25, 2023

I just ran into this, sadly it seems to break almost all links in my documentation so this is a really high impact bug in my opinion. Especially since with the GitHub mirror it works fine.

Here's a demo file with a link that should work but it doesn't:

https://try.gitea.io/blablablablab/BlaTest/src/branch/main/A%20B/outsidelink.md

(The big bold (test link) is what you want to click. Expected behavior is that it gets you to the target file in the repository root, actual behavior is some nonsensical location.)

@vilunov
Copy link

vilunov commented Jan 29, 2024

In original post it is said that relative links such as [text](file.md) or [text](./file.md) resolve correctly, but absolute ones do not. This is not my experience with Gitea 1.21.4. Example (this was fixed by downgrading to 1.21.3)

I expect that a link such as [Project structure](crates/readme.md) in the project readme should be resolved to example.com/org/repo/src/branch/main/crates/readme.md, instead it is resolved to example.com/org/repo/crates/readme.md, making it pretty much useless, as it is always broken.

@vilunov
Copy link

vilunov commented Jan 30, 2024

The issue above seems to be a regression between 1.21.3 and 1.21.4, probably even by #28803
Fortunately, there was no migration in 1.21.4, it was easy to downgrade one patch version

@LamaBleu
Copy link

Thank you so much @vilunov, you saved my day. Downgrade to 1.21.3 solved this annoying issue.

@vilunov
Copy link

vilunov commented Jan 30, 2024

@KN4CK3R sorry, pinging for visibility since you made the MR and you seem to be responsible for that area.

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 30, 2024

Can't reproduce it on current main. Was it fixed by #28909?

grafik

@vilunov
Copy link

vilunov commented Jan 31, 2024

Thanks, sorry for panicking. Relative links work again, but the absolute ones are still broken.

A link such as /docs/running.md will be rendered as it is in html and will lead to localhost:3000/docs/running.md

image
image

This is on gitea/gitea:1.21-nightly docker image

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 31, 2024

Ok, that's not a regression of my PR 🎉 Gitea always rendered them like that. It's debatable what the root is for absolute links. Even Github is inconsistent with the links.

Inside issue:
A

/A

../A

../../A

../../../A

../../../../A

../../../../../A

../../../../../../A

/../A

Inside repo:
https://github.com/KN4CK3R/Test/blob/4398ccfc630b9b438421ca7e16c60de0538127cd/README.md


A fix would be to strip the / prefix for links too:

link = []byte(giteautil.URLJoin(base, string(link)))

- link = []byte(giteautil.URLJoin(base, string(link)))
+ link = []byte(giteautil.URLJoin(base, strings.TrimLeft(string(link), "/")))

@vilunov
Copy link

vilunov commented Jan 31, 2024

GitHub is not very consistent even when it comes to rendering README.md as a repo front page or as an individual file.

Example:
link in the first footnote at the end of README.md
https://github.com/ratijas/kdesrc-build-sublime -- renders as https://github.com/ratijas/plugins/gen_conf_options.py
https://github.com/ratijas/kdesrc-build-sublime/blob/master/README.md -- renders as https://github.com/ratijas/kdesrc-build-sublime/blob/master/plugins/gen_conf_options.py

It really is tricky

@ell1e
Copy link

ell1e commented Mar 24, 2024

@vilunov the first one seems more like a bug, doesn't it? usually in almost all places on GitHub it behaves like the 2nd example. maybe they just forgot about this corner case of footnotes in the one location, that feels like the most likely reason.

on gitea, sadly it still doesn't seem to work for links starting with / ever, not even in some cases, see "Broken Example 2" here. this makes writing documentation where individual pages can be moved around inside the documentation folder without all their links to other files breaking quite difficult. so i hope this can be fixed eventually.

edit: this also can't be just fixed on the user side by leaving the / away, since when your documentation page is in various sub folders and links to neighboring pages but in other subfolders, you will need to use tons of .. to climb out of where your current doc page is when omitting / (= all outgoing links in the page break when the page is moved around in your repository) while with / it's always relative to the repository base (= a doc page can be moved around without all its outgoing links to others breaking). on github this works perfectly fine, on gitea it doesn't. this use case is also not relevant outside of markdown files inside the repo for documentation, so that e.g. github doesn't also apply this behavior in issue tickets makes sense in my humble opinion.

@levicki
Copy link

levicki commented May 1, 2024

I am having (a variation of?) this problem on the latest version of Gitea (served at root, not at sub-path) — For example I write:

see [README](README.md)

In the Release description field (which works fine on Github and Gitlab) and the link is replaced with:

https://example.com:3000/my_organization/my_project/README.md

And results in 404 error when clicked. Actual README.md link in the source tree shows as:

https://example.com:3000/my_organization/my_project/src/branch/master/README.md

I don't think either would be correct for Releases though — it should link to specific tagged release version of the file.

The only way I can produce a correct link is if I do it manually like this:

see [README](src/tag/1.0.0/README.md)

Which results in:

https://example.com:3000/my_organization/my_project/src/tag/1.0.0/README.md

It would be nice if Gitea was smart enough to do that sort of expansion for me if I provide a relative path without leading / or ./.

@neutronstriker
Copy link

Is this bug resolved now, I am facing the same issue with version 1.21.4.
-Thanks.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 22, 2024

It is not really fixed. Because the "absolute path" link (starting with a slash /xxxx) handling is quite tricky and ambiguous.

And, if we change this behavior, it is a "breaking" change and will make all existing "absolute-path" links broken.

@neutronstriker
Copy link

It is not really fixed. Because the "absolute path" link (starting with a slash /xxxx) handling is quite tricky and ambiguous.

And, if we change this behavior, it is a "breaking" change and will make all existing "absolute-path" links broken.

Which means we need to use /src/branch/main/ as a prefix right?

@wxiaoguang
Copy link
Contributor

Which means we need to use /src/branch/main/ as a prefix right?

Or use relative path like ../../foo/bar.


Maybe I could take a try in 1.23 to "fix/improve" the path handling, but I can't promise at the moment.

@wxiaoguang wxiaoguang self-assigned this Jun 22, 2024
@busslina
Copy link

busslina commented Jun 22, 2024

I moved recently to Forgejo, based on Gitea, and works fine.
Even MD header URL links works fine, which I think is not the same with Gitea.

@neutronstriker
Copy link

neutronstriker commented Jun 22, 2024

I just updated my installation to 1.22.0 and my issue got resolved, my issue was the 1st case referenced in comment #18592 (comment)

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 22, 2024

I moved recently to Forgejo, based on Gitea, and works fine. Even MD header URL links works fine, which I think is not the same with Gitea.

There are some different path problems, some have been fixed, while some are still not. Pretty sure they do not have the proper fix either.

Forgejo is still largely based on Gitea's work, but they skipped some Gitea's commits (even some are security related) so I guess it couldn't be more complete than Gitea.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 22, 2024

Made some tests on GitHub, I think we could just copy the behavior (and mark it as breaking)

https://github.com/wxiaoguang/playground/blob/44cc37ef49eb827345dd0d9d510b319f81f63f16/test.md

https://github.com/wxiaoguang/playground/blob/6bd6f1da05afc914abf452628d90c90b71093820/dir/test.md

Actually, the rule is simple:

  • absolute path: from current git ref
  • relative path: from current dir

But not sure whether we should 100% follow it ........

Context Relative Link
In a .md file on the same branch /assets/images/electrocat.png
In a .md file on another branch /../main/assets/images/electrocat.png
In issues, pull requests and comments of the repository ../blob/main/assets/images/electrocat.png?raw=true
In a .md file in another repository /../../../../github/docs/blob/main/assets/images/electrocat.png
In issues, pull requests and comments of another repository ../../../github/docs/blob/main/assets/images/electrocat.png?raw=true

@levicki
Copy link

levicki commented Jun 22, 2024

But not sure whether we should 100% follow it ........

I vote for using the same logic as GitHub — that makes it easier to have both private Gitea and public GitHub repo with correct links in both.

Also please make sure that it works not just in issues and comments but in releases as well. Thanks for looking into it and taking time to fix it.

@ell1e

This comment was marked as off-topic.

@wxiaoguang

This comment was marked as off-topic.

@delvh

This comment was marked as off-topic.

@busslina

This comment was marked as off-topic.

@wxiaoguang

This comment was marked as off-topic.

@ApplePie420
Copy link

v1.21.4, bare metal installation, server as root of subdomain. I have the same problem, e.g.

In README.md I have

[Schematics available in the docs folder](docs/schematics.pdf)

And it doesn't work. Full path to the file is src/branch/main/docs/schematics.pdf. This is extremely stupid, not only introduces way more text to typo, but just is not intuitive and transferable to other git UIs. I assume this hasn't been resolved yet.

@lunny lunny added the type/bug label Nov 20, 2024
@lunny
Copy link
Member

lunny commented Nov 21, 2024

v1.21.4, bare metal installation, server as root of subdomain. I have the same problem, e.g.

In README.md I have

[Schematics available in the docs folder](docs/schematics.pdf)

And it doesn't work. Full path to the file is src/branch/main/docs/schematics.pdf. This is extremely stupid, not only introduces way more text to typo, but just is not intuitive and transferable to other git UIs. I assume this hasn't been resolved yet.

Can you upgrade to v1.22.3 to try it again?

@wxiaoguang
Copy link
Contributor

Can you upgrade to v1.22.3 to try it again?

It hasn't been improved yet. I am still working on some markdown refactorings. If I complete I would tell here.

@wxiaoguang
Copy link
Contributor

Refactor markup render system #32612

Almost done, this PR will fix most problems.

@wxiaoguang wxiaoguang removed their assignment Nov 22, 2024
@wxiaoguang
Copy link
Contributor

v1.21.4, bare metal installation, server as root of subdomain. I have the same problem, e.g.

In README.md I have

[Schematics available in the docs folder](docs/schematics.pdf)

And it doesn't work. Full path to the file is src/branch/main/docs/schematics.pdf. This is extremely stupid, not only introduces way more text to typo, but just is not intuitive and transferable to other git UIs. I assume this hasn't been resolved yet.

Hi ApplePie420 , I have tested your markdown in 1.22, it seems that 1.22 could render that relative path correctly (something should have been fixed there). Would you like to upgrade to 1.22 and take a try? No need to wait for 1.23.

Screenshot:

image

If anything doesn't work as expected, please provide some reproducible examples. Thank you.

@ell1e
Copy link

ell1e commented Nov 23, 2024

https://demo.gitea.com/ell1e/linktest/src/branch/main/README.md This example suggests relative links with no trailing slash work, but relative links with trailing slash to link to files relative to the root of the file tree still don't work (which they do on GitHub). I hope this is helpful.

@wxiaoguang
Copy link
Contributor

https://demo.gitea.com/ell1e/linktest/src/branch/main/README.md This example suggests relative links with no trailing slash work, but relative links with trailing slash to link to files relative to the root of the file tree still don't work (which they do on GitHub). I hope this is helpful.

Thank you, that's another problem and will be fixed in 1.23 (see my PR #32612)

I think it works as expected now, feel free to try that PR 🙏

image

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 24, 2024

#32612 has been merged and the nightly builds are ready (docker: main-nightly, binary: https://dl.gitea.com/gitea/main-nightly/ ), feel free to try it.

I think the "relative/absolute" path problem should have been fixed for most cases.

I have done my best to avoid regressions, if there is anything wrong, please let me know and I will fix bugs in first time.


Update: found a regression Fix markup render regression and fix some tests #32640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.