mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-08-20 06:06:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2301 from m-martin-78/website-urlencoded-chars

Browse Source 
Bugfix: url-encoded chars on the website listener
This commit is contained in:
Ben McClelland
2026-08-19 14:32:36 -07:00
committed by GitHub
parent f9dc02409c a40aae87c4
commit 417c84fc4d
4 changed files with 251 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/integration/WebsiteHosting.go
+52
View File
@@ -862,3 +862,55 @@ func WebsiteHosting_options_preflight_missing_origin(s *S3Conf) error {
return checkWebsiteErrorResponse(resp, s3err.GetAPIError(s3err.ErrMissingCORSOrigin))
})
}
// WebsiteHosting_url_encoded_object_key tests that object keys containing
// characters requiring percent-encoding are served, and that a key holding a
// literal percent sign is not decoded twice.
func WebsiteHosting_url_encoded_object_key(s *S3Conf) error {
testName := "WebsiteHosting_url_encoded_object_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
err := putBucketWebsiteConfig(s3client, bucket, &types.WebsiteConfiguration{
IndexDocument: &types.IndexDocument{
Suffix: getPtr("index.html"),
},
})
if err != nil {
return err
}
if err := grantPublicBucketPolicy(s3client, bucket, policyTypeObject); err != nil {
return err
}
for _, test := range []struct {
key string
path string
}{
{"my file.html", "/my%20file.html"},
{"my dir/index.html", "/my%20dir/"},
{"café.html", "/caf%C3%A9.html"},
{"a%20b.html", "/a%2520b.html"},
} {
content := "<html><body>" + test.key + "</body></html>"
_, err = putObjectWithData(int64(len(content)), &s3.PutObjectInput{
Bucket: &bucket,
Key: &test.key,
Body: strings.NewReader(content),
ContentType: getPtr("text/html"),
}, s3client)
if err != nil {
return err
}
resp, err := websiteGet(s, bucket, test.path, nil)
if err != nil {
return err
}
if err := checkWebsiteResponse(resp, http.StatusOK, []byte(content)); err != nil {
return fmt.Errorf("%s: %w", test.path, err)
}
}
return nil
})
}
tests/integration/group-tests.go
+2
View File
@@ -717,6 +717,7 @@ func TestWebsiteHosting(ts *TestState) {
ts.Run(WebsiteHosting_options_preflight_access_granted)
ts.Run(WebsiteHosting_options_preflight_access_forbidden)
ts.Run(WebsiteHosting_options_preflight_missing_origin)
ts.Run(WebsiteHosting_url_encoded_object_key)
}
func TestPreflightOPTIONSEndpoint(ts *TestState) {
@@ -1892,6 +1893,7 @@ func GetIntTests() IntTests {
"WebsiteHosting_options_preflight_access_granted": WebsiteHosting_options_preflight_access_granted,
"WebsiteHosting_options_preflight_access_forbidden": WebsiteHosting_options_preflight_access_forbidden,
"WebsiteHosting_options_preflight_missing_origin": WebsiteHosting_options_preflight_missing_origin,
"WebsiteHosting_url_encoded_object_key": WebsiteHosting_url_encoded_object_key,
"PreflightOPTIONS_non_existing_bucket": PreflightOPTIONS_non_existing_bucket,
"PreflightOPTIONS_missing_origin": PreflightOPTIONS_missing_origin,
"PreflightOPTIONS_invalid_request_method": PreflightOPTIONS_invalid_request_method,
website/handler.go
+34 -10
View File
@@ -19,6 +19,7 @@ import (
"fmt"
"io"
"net/http"
"net/url"
"strconv"
"strings"
@@ -125,12 +126,28 @@ func (c *websiteController) Options(ctx fiber.Ctx) error {
func registerWebsiteRoutes(app *fiber.App, be backend.Backend, domain string) {
controller := newWebsiteController(be, domain)
// percent-decode the request path, so object keys containing special
// characters resolve to the actual key. The s3 api and admin servers
// mount the same middleware.
app.Use("*", decodeURL)
app.Head("*", controller.Head)
app.Get("*", controller.Get)
app.Options("*", controller.Options)
app.All("*", controller.MethodNotAllowed)
}
// decodeURL wraps the shared DecodeURL middleware to continue the chain and to
// report malformed percent-encoding as an html error page.
func decodeURL(ctx fiber.Ctx) error {
if err := middlewares.DecodeURL(ctx); err != nil {
debuglogger.Logf("failed to unescape the request path: %v", err)
return sendError(ctx, s3err.GetAPIError(s3err.ErrInvalidURI))
}
return ctx.Next()
}
func setCORSPreflightHeaders(ctx fiber.Ctx, allowConfig *auth.CORSAllowanceConfig) {
ctx.Set("Access-Control-Allow-Origin", allowConfig.Origin)
ctx.Set("Access-Control-Allow-Methods", allowConfig.Methods)
@@ -195,8 +212,6 @@ func (c *websiteController) resolveRequest(ctx fiber.Ctx) (*websiteRequestInfo,
return nil, err
}
fmt.Println(bucket)
key := strings.TrimPrefix(ctx.Path(), "/")
if err := validateWebsiteNames(bucket, key); err != nil {
return nil, err
@@ -411,10 +426,8 @@ func handleRedirectAll(ctx fiber.Ctx, redirect *s3response.RedirectAllRequestsTo
protocol = ctx.Scheme()
}
location := fmt.Sprintf("%s://%s/%s", protocol, redirect.HostName, key)
if query := string(ctx.Request().URI().QueryString()); query != "" {
location += "?" + query
}
location := websiteLocation(protocol, redirect.HostName, key,
string(ctx.Request().URI().QueryString()))
return sendRedirect(ctx, http.StatusMovedPermanently, location)
}
@@ -444,13 +457,24 @@ func applyRedirect(ctx fiber.Ctx, redirect *s3response.Redirect, condition *s3re
}
}
location := fmt.Sprintf("%s://%s/%s", protocol, host, key)
if query := string(ctx.Request().URI().QueryString()); query != "" {
location += "?" + query
}
location := websiteLocation(protocol, host, key,
string(ctx.Request().URI().QueryString()))
return sendRedirect(ctx, httpCode, location)
}
// websiteLocation builds a redirect target url. The object key is
// percent-encoded, as the keys reaching this point are decoded.
func websiteLocation(protocol, host, key, query string) string {
location := url.URL{
Scheme: protocol,
Host: host,
Path: "/" + key,
RawQuery: query,
}
return location.String()
}
func sendRedirect(ctx fiber.Ctx, statusCode int, location string) error {
ctx.Set("Location", location)
_, _ = utils.EnsureRequestIDs(ctx)
website/handler_test.go
+163
View File
@@ -27,6 +27,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/gofiber/fiber/v3"
"github.com/valyala/fasthttp"
"github.com/versity/versitygw/auth"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/s3err"
@@ -44,6 +45,7 @@ type websiteTestBackend struct {
objectErrors map[string]error
public bool
calls []string
objectKeys []string
}
func (b *websiteTestBackend) record(call string) {
@@ -96,6 +98,7 @@ func (b *websiteTestBackend) HeadObject(_ context.Context, input *s3.HeadObjectI
if input == nil || input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
b.objectKeys = append(b.objectKeys, *input.Key)
if err, ok := b.objectErrors[*input.Key]; ok {
return nil, err
}
@@ -119,6 +122,7 @@ func (b *websiteTestBackend) GetObject(_ context.Context, input *s3.GetObjectInp
if input == nil || input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
b.objectKeys = append(b.objectKeys, *input.Key)
if err, ok := b.objectErrors[*input.Key]; ok {
return nil, err
}
@@ -970,6 +974,165 @@ func TestWebsiteHandlerMethodNotAllowed(t *testing.T) {
}
}
func TestWebsiteHandlerDecodesURLEncodedPath(t *testing.T) {
tests := []struct {
name string
method string
path string
wantKey string
}{
{
name: "space in the object key",
method: http.MethodGet,
path: "/my%20file.html",
wantKey: "my file.html",
},
{
name: "space in the directory prefix resolves the index document",
method: http.MethodGet,
path: "/my%20dir/",
wantKey: "my dir/index.html",
},
{
name: "utf8 multibyte characters",
method: http.MethodHead,
path: "/caf%C3%A9.html",
wantKey: "café.html",
},
{
name: "literal plus sign is not turned into a space",
method: http.MethodGet,
path: "/c++notes.html",
wantKey: "c++notes.html",
},
{
name: "percent in the object name is decoded only once",
method: http.MethodGet,
path: "/a%2520b.txt",
wantKey: "a%20b.txt",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
body := "content of " + tt.wantKey
be := newWebsiteTestBackend(t, s3response.WebsiteConfiguration{
IndexDocument: &s3response.IndexDocument{Suffix: "index.html"},
}, map[string]string{tt.wantKey: body}, true)
resp := websiteRequestWithMethod(t, be, tt.method, tt.path)
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("status = %d, want %d (requested keys: %v)",
resp.StatusCode, http.StatusOK, be.objectKeys)
}
if len(be.objectKeys) != 1 || be.objectKeys[0] != tt.wantKey {
t.Fatalf("requested keys = %v, want [%q]", be.objectKeys, tt.wantKey)
}
if tt.method != http.MethodGet {
return
}
if got := readBody(t, resp); got != body {
t.Fatalf("body = %q, want %q", got, body)
}
})
}
}
// TestWebsiteHandlerEncodedPathTraversalIsRejected covers the validation
// hardening the path decoding brings: before decoding, the literal "%2E%2E%2F"
// contained no "..", so it passed the object name validation.
func TestWebsiteHandlerEncodedPathTraversalIsRejected(t *testing.T) {
be := newWebsiteTestBackend(t, s3response.WebsiteConfiguration{
IndexDocument: &s3response.IndexDocument{Suffix: "index.html"},
}, nil, true)
resp := websiteRequest(t, be, "/%2E%2E%2Fprivate.html")
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusBadRequest)
}
if len(be.objectKeys) != 0 {
t.Fatalf("traversal should not reach the backend, got keys: %v", be.objectKeys)
}
}
// TestWebsiteHandlerInvalidPercentEncoding exercises the decodeURL middleware
// directly: net/http refuses to build a request with a malformed escape, so
// such a path can only come from a raw client.
func TestWebsiteHandlerInvalidPercentEncoding(t *testing.T) {
fctx := &fasthttp.RequestCtx{}
fctx.Request.SetRequestURI("/%zz")
ctx := fiber.New().AcquireCtx(fctx)
if err := decodeURL(ctx); err != nil {
t.Fatalf("decodeURL returned an error: %v", err)
}
if got := ctx.Response().StatusCode(); got != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", got, http.StatusBadRequest)
}
if got := string(ctx.Response().Header.Peek("x-amz-error-code")); got != "InvalidURI" {
t.Fatalf("x-amz-error-code = %q, want %q", got, "InvalidURI")
}
}
func TestWebsiteHandlerRedirectLocationIsEncoded(t *testing.T) {
tests := []struct {
name string
config s3response.WebsiteConfiguration
path string
wantLocation string
}{
{
name: "RedirectAllRequestsTo keeps the key encoded",
config: s3response.WebsiteConfiguration{
RedirectAllRequestsTo: &s3response.RedirectAllRequestsTo{
HostName: "target.test",
Protocol: "https",
},
},
path: "/my%20file.html",
wantLocation: "https://target.test/my%20file.html",
},
{
name: "routing rule prefix replacement keeps the suffix encoded",
config: s3response.WebsiteConfiguration{
IndexDocument: &s3response.IndexDocument{Suffix: "index.html"},
RoutingRules: []s3response.RoutingRule{
{
Condition: &s3response.RoutingRuleCondition{
KeyPrefixEquals: "old dir/",
},
Redirect: &s3response.Redirect{
ReplaceKeyPrefixWith: "new dir/",
},
},
},
},
path: "/old%20dir/my%20file.html",
wantLocation: "http://site.test/new%20dir/my%20file.html",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
be := newWebsiteTestBackend(t, tt.config, nil, true)
resp := websiteRequest(t, be, tt.path)
defer resp.Body.Close()
if resp.StatusCode != http.StatusMovedPermanently {
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusMovedPermanently)
}
if got := resp.Header.Get("Location"); got != tt.wantLocation {
t.Fatalf("Location = %q, want %q", got, tt.wantLocation)
}
})
}
}
func newWebsiteTestBackend(t *testing.T, config s3response.WebsiteConfiguration, objects map[string]string, public bool) *websiteTestBackend {
t.Helper()