{http_server,url}.go: Fix redirects

r.URL.Path contains URL segments already decoded which makes it
impossible to distinguish from an encoded %2F and a path separator /.
We introduce redirect_with_slash which checks it properly.

There is still an occurence of r.URL.Path in the commit handler, but
that's going to get a major revamp anyway so I'm not bothering to fix
that for now. :/

2 files changed, 30 insertions, 16 deletions

diff --git a/http_server.go b/http_server.go
index 346ace3..e830fb1 100644
--- a/http_server.go
+++ b/http_server.go
@@ -32,8 +32,7 @@ func (router *http_router_t) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		if len(segments) < 2 {
 			http.Error(w, "Blank system endpoint", http.StatusNotFound)
 			return
-		} else if len(segments) == 2 && !trailing_slash {
-			http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
+		} else if len(segments) == 2 && redirect_with_slash(w, r) {
 			return
 		}
 
@@ -94,8 +93,7 @@ func (router *http_router_t) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	case non_empty_last_segments_len == separator_index+1:
 		http.Error(w, "Group root hasn't been implemented yet", http.StatusNotImplemented)
 	case non_empty_last_segments_len == separator_index+2:
-		if !trailing_slash {
-			http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
+		if redirect_with_slash(w, r) {
 			return
 		}
 		module_type := segments[separator_index+1]
@@ -124,10 +122,9 @@ func (router *http_router_t) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			}
 			// TODO: subgroups
 			if non_empty_last_segments_len == separator_index+3 {
-				if !trailing_slash {
-					http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
-					return
-				}
+		if redirect_with_slash(w, r) {
+			return
+		}
 				handle_repo_index(w, r, params)
 				return
 			}
@@ -137,15 +134,13 @@ func (router *http_router_t) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 				handle_repo_info(w, r, params)
 			case "tree":
 				params["rest"] = strings.Join(segments[separator_index+4:], "/")
-				if len(segments) < separator_index+5 {
-					http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
-					return
+				if len(segments) < separator_index+5 && redirect_with_slash(w, r) {
+			return
 				}
 				handle_repo_tree(w, r, params)
 			case "raw":
 				params["rest"] = strings.Join(segments[separator_index+4:], "/")
-				if len(segments) < separator_index+5 {
-					http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
+				if len(segments) < separator_index+5 && redirect_with_slash(w, r) {
 					return
 				}
 				handle_repo_raw(w, r, params)
@@ -154,14 +149,14 @@ func (router *http_router_t) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 					http.Error(w, "Too many parameters", http.StatusBadRequest)
 					return
 				}
-				if !trailing_slash {
-					http.Redirect(w, r, r.URL.Path+"/", http.StatusSeeOther)
+				if redirect_with_slash(w, r) {
 					return
 				}
 				handle_repo_log(w, r, params)
 			case "commit":
 				if trailing_slash {
 					http.Redirect(w, r, strings.TrimSuffix(r.URL.Path, "/"), http.StatusSeeOther)
+					// TODO
 					return
 				}
 				params["commit_id"] = segments[separator_index+4]
diff --git a/url.go b/url.go
index 86a3c6e..7712639 100644
--- a/url.go
+++ b/url.go
@@ -11,7 +11,7 @@ import (
 
 var (
 	err_duplicate_ref_spec = errors.New("Duplicate ref spec")
-	err_no_ref_spec        = errors.New("No ref spec")
+	err_no_ref_spec		= errors.New("No ref spec")
 )
 
 func get_param_ref_and_type(r *http.Request) (ref_type, ref string, err error) {
@@ -63,3 +63,22 @@ func parse_request_uri(request_uri string) (segments []string, params url.Values
 
 	return
 }
+
+func redirect_with_slash(w http.ResponseWriter, r *http.Request) bool {
+	request_uri := r.RequestURI
+
+	path_end := strings.IndexAny(request_uri, "?#")
+	var path, rest string
+	if path_end == -1 {
+		path = request_uri
+	} else {
+		path = request_uri[:path_end]
+		rest = request_uri[path_end:]
+	}
+
+	if !strings.HasSuffix(path, "/") {
+		http.Redirect(w, r, path+"/"+rest, http.StatusSeeOther)
+		return true
+	}
+	return false
+}