From da0eb5037ea0c818a74481cbbabdac32d2c24b8b Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 8 Oct 2023 10:40:08 +0200 Subject: [PATCH] httpd: skip StripSlash middleware for URL ending with multiple slashes Fixes #1434 Signed-off-by: Nicola Murino --- internal/httpd/httpd_test.go | 76 +++++++++++++++++++++++++++++++++++- internal/httpd/server.go | 10 +++-- 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 7f964fe2..143e0413 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -13940,7 +13940,8 @@ func TestShareUploadSingle(t *testing.T) { assert.NoError(t, err) req.SetBasicAuth(defaultUsername, defaultPassword) rr = executeRequest(req) - checkResponseCode(t, http.StatusNotFound, rr) + checkResponseCode(t, http.StatusBadRequest, rr) + assert.Contains(t, rr.Body.String(), "operation unsupported") err = os.MkdirAll(filepath.Join(user.GetHomeDir(), "dir"), os.ModePerm) assert.NoError(t, err) @@ -13951,6 +13952,13 @@ func TestShareUploadSingle(t *testing.T) { checkResponseCode(t, http.StatusBadRequest, rr) assert.Contains(t, rr.Body.String(), "operation unsupported") + // only uploads to the share root dir are allowed + req, err = http.NewRequest(http.MethodPost, path.Join(sharesPath, objectID, "dir", "file.dat"), bytes.NewBuffer(content)) + assert.NoError(t, err) + req.SetBasicAuth(defaultUsername, defaultPassword) + rr = executeRequest(req) + checkResponseCode(t, http.StatusNotFound, rr) + share, err = dataprovider.ShareExists(objectID, user.Username) assert.NoError(t, err) assert.Equal(t, 2, share.UsedTokens) @@ -22824,6 +22832,72 @@ func TestWebRole(t *testing.T) { assert.NoError(t, err) } +func TestNameParamSingleSlash(t *testing.T) { + err := dataprovider.Close() + assert.NoError(t, err) + err = config.LoadConfig(configDir, "") + assert.NoError(t, err) + providerConf := config.GetProviderConf() + providerConf.NamingRules = 5 + err = dataprovider.Initialize(providerConf, configDir, true) + assert.NoError(t, err) + + webToken, err := getJWTWebTokenFromTestServer(defaultTokenAuthUser, defaultTokenAuthPass) + assert.NoError(t, err) + apiToken, err := getJWTAPITokenFromTestServer(defaultTokenAuthUser, defaultTokenAuthPass) + assert.NoError(t, err) + csrfToken, err := getCSRFToken(httpBaseURL + webLoginPath) + assert.NoError(t, err) + group := getTestGroup() + group.Name = "/" + form := make(url.Values) + form.Set("name", group.Name) + form.Set("description", group.Description) + form.Set("max_sessions", "0") + form.Set("quota_files", "0") + form.Set("quota_size", "0") + form.Set("upload_bandwidth", "0") + form.Set("download_bandwidth", "0") + form.Set("upload_data_transfer", "0") + form.Set("download_data_transfer", "0") + form.Set("total_data_transfer", "0") + form.Set("max_upload_file_size", "0") + form.Set("default_shares_expiration", "0") + form.Set("max_shares_expiration", "0") + form.Set("password_expiration", "0") + form.Set("password_strength", "0") + form.Set("expires_in", "0") + form.Set("external_auth_cache_time", "0") + form.Set(csrfFormToken, csrfToken) + b, contentType, err := getMultipartFormData(form, "", "") + assert.NoError(t, err) + req, err := http.NewRequest(http.MethodPost, webGroupPath, &b) + assert.NoError(t, err) + req.Header.Set("Content-Type", contentType) + setJWTCookieForReq(req, webToken) + rr := executeRequest(req) + checkResponseCode(t, http.StatusSeeOther, rr) + + groupGet, _, err := httpdtest.GetGroupByName(group.Name, http.StatusOK) + assert.NoError(t, err) + assert.Equal(t, "/", groupGet.Name) + // cleanup + req, err = http.NewRequest(http.MethodDelete, groupPath+"/"+url.PathEscape(group.Name), nil) + assert.NoError(t, err) + setBearerForReq(req, apiToken) + rr = executeRequest(req) + checkResponseCode(t, http.StatusOK, rr) + + err = dataprovider.Close() + assert.NoError(t, err) + err = config.LoadConfig(configDir, "") + assert.NoError(t, err) + providerConf = config.GetProviderConf() + providerConf.BackupsPath = backupsPath + err = dataprovider.Initialize(providerConf, configDir, true) + assert.NoError(t, err) +} + func TestAddWebGroup(t *testing.T) { webToken, err := getJWTWebTokenFromTestServer(defaultTokenAuthUser, defaultTokenAuthPass) assert.NoError(t, err) diff --git a/internal/httpd/server.go b/internal/httpd/server.go index b28cdcfc..14722707 100644 --- a/internal/httpd/server.go +++ b/internal/httpd/server.go @@ -1164,7 +1164,9 @@ func (s *httpdServer) redirectToWebPath(w http.ResponseWriter, r *http.Request, } } -func (s *httpdServer) isStaticFileURL(r *http.Request) bool { +// The StripSlashes causes infinite redirects at the root path if used with http.FileServer. +// We also don't strip paths with more than one trailing slash, see #1434 +func (s *httpdServer) mustStripSlash(r *http.Request) bool { var urlPath string rctx := chi.RouteContext(r.Context()) if rctx != nil && rctx.RoutePath != "" { @@ -1172,7 +1174,8 @@ func (s *httpdServer) isStaticFileURL(r *http.Request) bool { } else { urlPath = r.URL.Path } - return !strings.HasPrefix(urlPath, webOpenAPIPath) && !strings.HasPrefix(urlPath, webStaticFilesPath) + return !strings.HasSuffix(urlPath, "//") && !strings.HasPrefix(urlPath, webOpenAPIPath) && + !strings.HasPrefix(urlPath, webStaticFilesPath) && !strings.HasPrefix(urlPath, acmeChallengeURI) } func (s *httpdServer) initializeRouter() { @@ -1221,8 +1224,7 @@ func (s *httpdServer) initializeRouter() { s.router.Use(c.Handler) } s.router.Use(middleware.GetHead) - // StripSlashes causes infinite redirects at the root path if used with http.FileServer - s.router.Use(middleware.Maybe(middleware.StripSlashes, s.isStaticFileURL)) + s.router.Use(middleware.Maybe(middleware.StripSlashes, s.mustStripSlash)) s.router.NotFound(s.notFoundHandler)