Skip to content

Markdown links started with / broken in 1.23 #33274

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

Closed
didim99 opened this issue Jan 14, 2025 · 14 comments · Fixed by #34114
Closed

Markdown links started with / broken in 1.23 #33274

didim99 opened this issue Jan 14, 2025 · 14 comments · Fixed by #34114

Comments

@didim99
Copy link

didim99 commented Jan 14, 2025

Description

If you paste a link tag in any place (MD files, issues, comments) started with leading / (without domain) it don't works anymore in 1.23. I'm not sure about 1.22.5 or 1.22.6 but for 1.22.4 or older version this kind of links works perfectly: parsed as absolute URL for whole Gitea instance (domain). For now these liks are parsed as relative to current repository.

[works](https://yourdomain.com:9915/org/repo/readme#section)
[not works](/org/repo/readme#section) <-- works fine in 1.22.4

For example, if you want to link other repo now you need to specify full URL, including domain and port (if customized). In case of transfer Gitea instance to another domain/port it seems to be better to use absolute URLs without domain.

This change broken all our documentation system and old conversation in issues, please, fix it!

Gitea Version

1.23.1

Can you reproduce the bug on the Gitea demo site?

Yes
MD: https://demo.gitea.com/didim99/test-2/src/branch/master/readme.md
Issues: https://demo.gitea.com/didim99/test/issues/1#issue-798

Log Gist

No response

Screenshots

No response

Git Version

Not matters

Operating System

Not matters

How are you running Gitea?

Official install script (updated from 1.22.6)

Database

MySQL/MariaDB

@didim99 didim99 changed the title Relative links started with / broken in 1.23 Markdown links started with / broken in 1.23 Jan 14, 2025
@TheFox0x7
Copy link
Contributor

@wxiaoguang FYI since you refactored that part.

I think current behavior is more correct and mirroring github one. Though it wasn't marked as a breaking change.

@didim99
Copy link
Author

didim99 commented Jan 14, 2025

more correct and mirroring github one.

@TheFox0x7 I read this github blog post and related documentation page and can't find any information about links with leading / character. Nothing like "absolute links starting with / should not work". And in HTML standard that kind of URLs is correct.

Okay, relative links without leading / works as before (including ones that contains .. sequence), sounds really like "mirroring github", but there is nothing about leading /. It worked before for a years (at least since mid 2022) and don't works now. So I think it's a bug.

@TheFox0x7
Copy link
Contributor

Fair enough, I missed that there's no / in front in that github document. My apologies.
I'm not saying it's not a bug, it is a breaking change which went unannounced so it might as well be a bug/regression.
I did test relative links on github and it treats relative and absolute paths the same, see: https://github.com/TheFox0x7/repro/blob/main/linker.md

What I gather is that you want the /* links to not include the base repository like in the test here:
https://github.com/go-gitea/gitea/blob/main/modules/markup/render_link_test.go#L21
/a being base repository, b being current path in the repository (I'm guessing), and /c being the link from the markup content, which should resolve to /c instead of /a/c. Correct?

I think changing this line to be just link would cover that, though then the nested absolute path test fails and I'm not sure whats the purpose of the /owner/..../owner/.. case.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 15, 2025

Yup, it's not broken but just follows other git service's behavior like GitHub.

If you'd like to cross-reference the documents, it needs to use use full link (with https://host/ prefix)

Sorry for the "breaking" change, because there are more stronger requirements for it:


[works](https://yourdomain.com:9915/org/repo/readme#section)

ps: I see you are using "readme" for an org, actually there is a feature "profile readme", you could put the readme in {org}/.profile/README.md, then it will be rendered at the org's home page.

@wxiaoguang wxiaoguang added issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail and removed type/bug labels Jan 15, 2025
@didim99
Copy link
Author

didim99 commented Jan 15, 2025

/a being base repository, b being current path in the repository (I'm guessing), and /c being the link from the markup content, which should resolve to /c instead of /a/c. Correct?

@TheFox0x7 yep, and tests that you mentioned above written in opposite approach.

just follows other git service's behavior like GitHub

@wxiaoguang yes, that's true. But "as others" not always means "better", right? Previous approach was more flexible, allows choice between absolute and relative links using leading /, and for now no more choice.

My proposal is: if this is not too hard to implement, could you make this behaviour configurable via app.ini? Something like ui.use_absolute_links with default false (github-like) but with ability to setting true for backward compatibility for those who used this feature before.

I see you are using "readme" for an org

Readme is just an example. Actually all our cross-repo links (hundreds of them) such as code reference in issues, documentation and so on were written in that manner for portability reason (changing base domain (especially port number) of gitea instance do not broke these links).

@silverwind
Copy link
Member

silverwind commented Jan 15, 2025

This /org/repo/file.md is ambigous with in-repo file links like /file.md which work in both GitHub and Gitea. To really distinguish the two, an new non-ambiguous syntax would be needed, maybe prefix with // or something similar like protocol-relative URLs, but not exactly the same either.

@wxiaoguang
Copy link
Contributor

maybe prefix with // or something similar

I do not think // could work here, it only omits the "protocol" part.

@silverwind
Copy link
Member

silverwind commented Jan 15, 2025

Yea we'd need a "origin-relative" or "suburl-relative" URL, but there is no standard for that as far as I'm aware. Maybe /// 😆

@wxiaoguang wxiaoguang removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Jan 19, 2025
@wxiaoguang
Copy link
Contributor

Yea we'd need a "origin-relative" or "suburl-relative" URL, but there is no standard for that as far as I'm aware. Maybe /// 😆

Or maybe /./path-to-site-root? It won't break existing URLs and is a pretty valid URL

[not works](/./org/repo/readme#section) <-- proposal

@didim99 what do you think?

@didim99
Copy link
Author

didim99 commented Apr 3, 2025

r maybe /./path-to-site-root?

@wxiaoguang I'm agree, it may be a solution in terms of most possible compatibility.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 3, 2025

I am going to make some more improvements to the markdown module (to clean up some legacy problems). For this "relative link" problem, there are some initial ideas in my mind at the moment, let's see how good we can make it:

  • Using a config option: use_absolute_links=true
  • Using a special link path: /./path-to-site-root
  • Using some special names:
    • /@root/path-to-site-root, for example: /@root/AnotherOrg/TheRepo/issues
    • /@owner/a-repo-name, for example: /@owner/AnotherRepo/issues
    • /@repo/a-path-in-repo, for example: /@repo/src/branch/raw/...

The special names seem flexible enough, the only question is that whether it would cause unsolvable conflict in the future.

  • Will it conflict with @ mentioning parser?
    • Maybe use other chars like /$root/.... ?

@silverwind
Copy link
Member

silverwind commented Apr 3, 2025

We should only definitely use characters that are forbidden in repo and org names. I think @ is forbidden in both, but good to double-check as I recall gitea is still allowing too much and is not compatible with github regarding allowed repo and org names (see #4150 (comment) and #4150 (comment) specifically).

@didim99
Copy link
Author

didim99 commented Apr 3, 2025

@wxiaoguang using special names looks very nice! IMO using @ as marker char is a not best idea because that notation used for mentions, $ (or maybe something else, but for now I not have any suitable proposal) is more preferred in this case.

@wxiaoguang
Copy link
Contributor

-> Refactor markup render to fix various path problems #34114

The final decision in my mind is "using :"

The reason is:

  • : is a valid URL path character, no need to escape
  • : won't conflict with other designs (@ for mentioning, $ for variable, etc)

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

Successfully merging a pull request may close this issue.

4 participants