Skip to content

Commit

Permalink
httpd: skip StripSlash middleware for URL ending with multiple slashes
Browse files Browse the repository at this point in the history
Fixes #1434

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
  • Loading branch information
drakkan committed Oct 8, 2023
1 parent 4b685b2 commit da0eb50
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 5 deletions.
76 changes: 75 additions & 1 deletion internal/httpd/httpd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions internal/httpd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1164,15 +1164,18 @@ 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 != "" {
urlPath = rctx.RoutePath
} 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() {
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit da0eb50

Please sign in to comment.