escape filename when assemble URL

[sillyguodong]

Fixes: #22843

Cause:

gitea/services/repository/files/content.go

Line 161 in affdd40

selfURL, err := url.Parse(fmt.Sprintf("%s/contents/%s?ref=%s", repo.APIURL(), treePath, origRef))

Previously, we did not escape the "%" that might be in "treePath" when call "url.parse()".

This function will check whether "%" is the beginning of an escape character. Obviously, the "%" in the example (hello%mother.txt) is not that. So, the function will return a error.

Solution:

We can escape "treePath" by call "url.PathEscape()" function firstly.

Screenshot:

[wolfogre]

CI failure is related.

[zeripath]

🤦 I can't believe that I missed these when I went through the codebase looking for this kind of failure of escaping.

This needs to be backported and we should go back through the code, yet again, and ensure that there aren't more of these.

[sillyguodong]

🤦 I can't believe that I missed these when I went through the codebase looking for this kind of failure of escaping.

This needs to be backported and we should go back through the code, yet again, and ensure that there aren't more of these.

Do you want me to help you do it together?

[zeripath]

origRef needs to be url.QueryEscape()

Suggested change

	selfURL, err := url.Parse(fmt.Sprintf("%s/contents/%s?ref=%s", repo.APIURL(), util.PathEscapeSegments(treePath), origRef))
	selfURL, err := url.Parse(fmt.Sprintf("%s/contents/%s?ref=%s", repo.APIURL(), util.PathEscapeSegments(treePath), url.QueryEscape(origRef)))

[zeripath]

Suggested change

	downloadURL, err := url.Parse(fmt.Sprintf("%s/raw/%s/%s/%s", repo.HTMLURL(), refType, ref, util.PathEscapeSegments(treePath)))
	downloadURL, err := url.Parse(fmt.Sprintf("%s/raw/%s/%s/%s", repo.HTMLURL(), url.PathEscape(refType), util.PathEscapeSegments(ref), util.PathEscapeSegments(treePath)))

[wxiaoguang]

Does it work if ref contains / ? eg: ref="a/b", treePath="c/d", then there will be "/a/b/c/d", which is the ref part. Or, if actually the endpoint doesn't care about it, then there would be no problem.

[zeripath]

The ref should be escape path segments.

The code does the magic of handling the ref

[wxiaoguang]

Oh yup, that's getRefNameFromPath. By design if ref /a exists, there should be no ref /a/b, so it's safe.

[zeripath]

I think the ref name should always be URL safe but... It's really much better to just escape the thing

[zeripath]

Suggested change

	htmlURL, err := url.Parse(fmt.Sprintf("%s/src/%s/%s/%s", repo.HTMLURL(), refType, ref, util.PathEscapeSegments(treePath)))
	htmlURL, err := url.Parse(fmt.Sprintf("%s/src/%s/%s/%s", repo.HTMLURL(), url.PathEscape(refType), util.PathEscapeSegments(ref), util.PathEscapeSegments(treePath)))

[zeripath]

modules/lfs/endpoint.go:

func endpointFromLocalPath(path string) *url.URL {
	var slash string
	if abs, err := filepath.Abs(path); err == nil {
		if !strings.HasPrefix(abs, "/") {
			slash = "/"
		}
		path = abs
	}

	var gitpath string
	if filepath.Base(path) == ".git" {
		gitpath = path
		path = filepath.Dir(path)
	} else {
		gitpath = filepath.Join(path, ".git")
	}

	if _, err := os.Stat(gitpath); err == nil {
		path = gitpath
	} else if _, err := os.Stat(path); err != nil {
		return nil
	}

	path = fmt.Sprintf("file://%s%s", slash, filepath.ToSlash(path))

	u, _ := url.Parse(path)

	return u
}

	path = fmt.Sprintf("file://%s%s", slash, filepath.ToSlash(path))

Needs to become:

	path = fmt.Sprintf("file://%s%s", slash, util.PathEscapeSegments(filepath.ToSlash(path)))

[zeripath]

If you can find any others that would be helpful.

[delvh]

Aren't we trying at the moment to get rid of HTMLURL by replacing it with Link?
Would that work here too?

[wxiaoguang]

Many HTMLURLs are for API or external systems (like LFS or submodules)

So, do not touch them at the moment. I have a full plan. Current progress: 50% after many PRs (some of them are listed in #22836), that's really a big project.

[wxiaoguang]

If you are interested in the HTMLURL refactoring, maybe you could help about #22861 😁 (update: deleted an uncertain word that I may misuse 😭 I mean "if you have time and interest")