Using / in Wiki Title break the wiki

[haskaalo]

In This Wiki Url For example it just show the Raw Markdown instead of the markdown with Gitea Layout.

[haskaalo]

Step to reproduce bug:

1. Create a page in a wiki on a repo (via browser)
2. Insert a slash inside title (I named mine GET: /a/page)
3. Create page.
4. Going to the page return raw page (in text)

[efy]

Had a quick look at this and it seems the route is matched on an unescaped path (A feature of the framework?).

So a request to /wiki/a%2Fb becomes /wiki/a/b which ends up matched by the wild card route that renders repo.WikiRaw.

[svarlamov]

This is also happening for paths with parenthesis in them, ie., Hello World (note) Is there a place where I can go in to help fix this?

[lunny]

It should be related with routes.

[svarlamov]

@lunny I briefly took a look at their docs: https://go-macaron.com/docs/middlewares/routing

Should we use a regex instead? Not sure why that URL-encoded string is not being handled the way that we want it (this is just from a few mins of looking at the code and haven't worked with macaron before, but...)

[svarlamov]

This is the correct router group for the wiki right? https://github.com/go-gitea/gitea/blob/master/routers/routes/routes.go#L596

[svarlamov]

I'm also wondering why it is that parentheses are not being handled... It feels like I'm missing something pretty fundamental about how Macaron parses routes...

[svarlamov]

Okay, so not saying I've got anything conclusive here, but I decided to test out macaron independently by running with the following little go file:

package main

import (
	"gopkg.in/macaron.v1"
)

func main() {
	m := macaron.Classic()
	m.Get("/foobar/:filename/hello",
		func(ctx *macaron.Context) string {
			return "You got it!"
		},
	)
	m.Run()
}

After starting that server (running on 0.0.0.0:4000 on my laptop), I tried out the following calls:

1. http://localhost:4000/foobar/file%20name/hello -> 200
2. http://localhost:4000/foobar/plain/hello -> 200
3. http://localhost:4000/foobar/url%2Fencoded%2Ftext%2Fwith%2Fslashes/hello -> 404

So it's looking like there's an issue with Macaron here...

@lunny What are your thoughts here? I spent a few mins looking through the macaron code (looks pretty simple), but I noticed you're not a contributor there...

From my work that I've outlined here so far, it seems that we have two options:

1. Contribute a fix and unit tests to Macaron and wait for that to get merged (who knows how long that will take?)
2. Refactor the routes such that we serve the page rather than the raw as a default (temporary band-aid while we await number 1 fix)

And, it's probably worth a deeper look if a bug like this has implications elsewhere in Gitea... I'm no expert on the Gitea codebase, but this sort of a routing issue could definitely have pretty far-reaching implications...

[lunny]

@svarlamov Thanks for your investigate, that's what I guess. Maybe you can send an issue or PR on Macaron and @unknwon is the owner of Macaron.

[svarlamov]

@lunny Sounds good. I will have a look there and maybe I will be able to create a PR instead of just an issue! 😛

Is the plan to stick with Macaron as the framework? It seems like it's a holdover from the gogs project... I'm a bit nervous to be running that in production -- would be cleaner to have gorilla mux or something like that... I assume you all have considered that?

In the meantime, are you okay to set the default route the Wiki HTML template though? I'm keen to contribute and get involved with this project :)

[lunny]

@svarlamov some maintainers suggested Gin instead of Macaron, but it seems that needs many work and maybe cause Gitea unstable. You are welcome to contribute to Gitea, please read CONTRIBUTING at first. And you can discuss about web framework or any plan on #develop channel on our discord server.

[lafriks]

I like go-chi router as it uses GoLang context features

[ethantkoenig]

@svarlamov I've created a PR to fix the problem in macaron: go-macaron/macaron#149

[svarlamov]

@ethantkoenig Thank you for the PR there, looking good... Hopefully @unknwon will be around to merge it there shortly 😄

[lafriks]

@ethantkoenig go-macaron PR has been merged 👍