Browse Source

httpd: skip StripSlash middleware for URL ending with multiple slashes

Fixes #1434

Signed-off-by: Nicola Murino <[email protected]>
Nicola Murino 2 years ago
parent
commit
da0eb5037e
2 changed files with 81 additions and 5 deletions
  1. 75 1
      internal/httpd/httpd_test.go
  2. 6 4
      internal/httpd/server.go

+ 75 - 1
internal/httpd/httpd_test.go

@@ -13940,7 +13940,8 @@ func TestShareUploadSingle(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NoError(t, err)
 	req.SetBasicAuth(defaultUsername, defaultPassword)
 	req.SetBasicAuth(defaultUsername, defaultPassword)
 	rr = executeRequest(req)
 	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)
 	err = os.MkdirAll(filepath.Join(user.GetHomeDir(), "dir"), os.ModePerm)
 	assert.NoError(t, err)
 	assert.NoError(t, err)
@@ -13951,6 +13952,13 @@ func TestShareUploadSingle(t *testing.T) {
 	checkResponseCode(t, http.StatusBadRequest, rr)
 	checkResponseCode(t, http.StatusBadRequest, rr)
 	assert.Contains(t, rr.Body.String(), "operation unsupported")
 	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)
 	share, err = dataprovider.ShareExists(objectID, user.Username)
 	assert.NoError(t, err)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, share.UsedTokens)
 	assert.Equal(t, 2, share.UsedTokens)
@@ -22824,6 +22832,72 @@ func TestWebRole(t *testing.T) {
 	assert.NoError(t, err)
 	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) {
 func TestAddWebGroup(t *testing.T) {
 	webToken, err := getJWTWebTokenFromTestServer(defaultTokenAuthUser, defaultTokenAuthPass)
 	webToken, err := getJWTWebTokenFromTestServer(defaultTokenAuthUser, defaultTokenAuthPass)
 	assert.NoError(t, err)
 	assert.NoError(t, err)

+ 6 - 4
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
 	var urlPath string
 	rctx := chi.RouteContext(r.Context())
 	rctx := chi.RouteContext(r.Context())
 	if rctx != nil && rctx.RoutePath != "" {
 	if rctx != nil && rctx.RoutePath != "" {
@@ -1172,7 +1174,8 @@ func (s *httpdServer) isStaticFileURL(r *http.Request) bool {
 	} else {
 	} else {
 		urlPath = r.URL.Path
 		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() {
 func (s *httpdServer) initializeRouter() {
@@ -1221,8 +1224,7 @@ func (s *httpdServer) initializeRouter() {
 		s.router.Use(c.Handler)
 		s.router.Use(c.Handler)
 	}
 	}
 	s.router.Use(middleware.GetHead)
 	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)
 	s.router.NotFound(s.notFoundHandler)