gitea: Extra slash in URL makes git clone fail

Description

Cloning a repository with a double slash fails:

$ git clone https://git.data.coop//halfd/new-website.git
Cloning into 'new-website'...
remote: Not found.
fatal: repository 'https://git.data.coop//halfd/new-website.git/' not found

Gitea Version

1.16.8

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Using the docker image.

Database

SQLite

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 15 (9 by maintainers)

Commits related to this issue

Support sanitising the URL by removing extra slashes in the URL (#21333) Changes in this PR : Strips incoming request URL of additional slashes (/). For example an input like `https://git.data... — committed to GiteaBot/gitea by sandyydk a year ago
Support sanitising the URL by removing extra slashes in the URL (#21333) (#23300) Backport #21333 Changes in this PR : Strips incoming request URL of additional slashes (/). For example an inp... — committed to go-gitea/gitea by GiteaBot a year ago

Most upvoted comments

This does work on GitHub, so I guess we can also allow one extra / at start of the path.

FYI, Github & Gitlab allows any arbitrary number of extra leading slashes, even better, it allows any number of slashes, for every valid slash position. Well I still consider this user fault, I personally won’t put up a PR for this. But feel free to use this patch and create a PR for this:

This is just to strip leading/trailing slashes and still have working behavior:

diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index 6ea1e1dfb..78e83abc6 100644
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -16,7 +16,7 @@ import (
        "code.gitea.io/gitea/modules/web/routing"
 
        "github.com/chi-middleware/proxy"
-       "github.com/go-chi/chi/v5/middleware"
+       "github.com/go-chi/chi/v5"
 )
 
 // Middlewares returns common middlewares
@@ -48,7 +48,38 @@ func Middlewares() []func(http.Handler) http.Handler {
                handlers = append(handlers, proxy.ForwardedHeaders(opt))
        }
 
-       handlers = append(handlers, middleware.StripSlashes)
+       // Strip leading and trailing slashes.
+       handlers = append(handlers, func(next http.Handler) http.Handler {
+               return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+                       var path string
+                       rctx := chi.RouteContext(req.Context())
+
+                       if rctx != nil && rctx.RoutePath != "" {
+                               path = rctx.RoutePath
+                       } else {
+                               path = req.URL.Path
+                       }
+
+                       if len(path) > 1 {
+                               // Strip trailing slashes.
+                               if strings.HasSuffix(path, "/") {
+                                       path = strings.TrimRight(path, "/")
+                               }
+
+                               // Strip double leading slashes to a single slash.
+                               if strings.HasPrefix(path, "//") {
+                                       path = "/" + strings.TrimLeft(path, "/")
+                               }
+
+                               if rctx == nil {
+                                       req.URL.Path = path
+                               } else {
+                                       rctx.RoutePath = path
+                               }
+                       }
+                       next.ServeHTTP(resp, req)
+               })
+       })

However if someone wants to go ahead and argue that also considering every other slash position to allow for multiple slashes, feel free to use this patch and create a PR:

diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index 6ea1e1dfb..64aa3603f 100644
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -16,7 +16,7 @@ import (
        "code.gitea.io/gitea/modules/web/routing"
 
        "github.com/chi-middleware/proxy"
-       "github.com/go-chi/chi/v5/middleware"
+       "github.com/go-chi/chi/v5"
 )
 
 // Middlewares returns common middlewares
@@ -48,7 +48,48 @@ func Middlewares() []func(http.Handler) http.Handler {
                handlers = append(handlers, proxy.ForwardedHeaders(opt))
        }
 
-       handlers = append(handlers, middleware.StripSlashes)
+       // Strip leading and trailing slashes.
+       handlers = append(handlers, func(next http.Handler) http.Handler {
+               return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+                       var path string
+                       rctx := chi.RouteContext(req.Context())
+
+                       if rctx != nil && rctx.RoutePath != "" {
+                               path = rctx.RoutePath
+                       } else {
+                               path = req.URL.Path
+                       }
+
+                       if len(path) > 1 {
+                               newPath := make([]rune, 0, len(path))
+
+                               prevWasSlash := false
+                               // Loop trough all characters and de-duplicate any multiple slashes to a single slash.
+                               for _, chr := range path {
+                                       if chr != '/' {
+                                               newPath = append(newPath, chr)
+                                               prevWasSlash = false
+                                               continue
+                                       }
+                                       if prevWasSlash {
+                                               continue
+                                       }
+                                       newPath = append(newPath, chr)
+                                       prevWasSlash = true
+                               }
+
+                               // Always remove the leading slash.
+                               modifiedPath := strings.TrimSuffix(string(newPath), "/")
+
+                               if rctx == nil {
+                                       req.URL.Path = modifiedPath
+                               } else {
+                                       rctx.RoutePath = modifiedPath
+                               }
+                       }
+                       next.ServeHTTP(resp, req)
+               })
+       })
 
        if !setting.DisableRouterLog {
                handlers = append(handlers, routing.NewLoggerHandler())

Gusted on Jul 23, 2022

OK, as a bystander, I think there are some misunderstanding or miscommunication during this issue’s history.

First, thank you all for making Gitea better.

The current situation is:

This issue is not resolved yet
There was already a PR trying to fix this problem: #21333
That PR already got some reviews, but there was no final conclusion yet.
A new PR #23163

In my mind:

It’s better to focus in #21333
Call TOC to make a final decision: go or no-go, with detailed explanations
ps: If it’s agreed to fix the double-slash, I think all double-slashes could be fixed together like 21333 (not only just fixing this issue), to make “//org//repo//…” work, like GitHub. Actually, GitHub supports any amount of slashes: https://github.com/go-gitea/gitea////////////tree/main/////.github

wxiaoguang on Feb 27, 2023