* s3: fix error mapping in GetObject to match HeadObject
The HeadObject path translates a 404 (Not Found) and a 405 (Method Not
Allowed) response into fs.ErrorObjectNotFound, but the GetObject path
returns the raw AWS SDK error instead. With --s3-no-head-object set,
Object.Open() is the first request to hit the source for a GET
operation, so callers that rely on fs.ErrorObjectNotFound see an
opaque "operation error S3: GetObject, ... StatusCode: 404" string
when --s3-no-head-object is set.
Mirror the error handling of HeadObject. The 405 (Method Not Allowed)
case applies to GetObject for the same reason it applies to
HeadObject: A request that specifies the versionId of a delete marker
returns 405 instead of 404 (commit 8470bdf810, AWS S3 delete-marker
docs).
Co-authored-by: Jan Schlien <rclone-github@jan-o-sch.net>
This commit is contained in:
@@ -4361,6 +4361,9 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
if statusCode := getHTTPStatusCode(err); statusCode == http.StatusNotFound || statusCode == http.StatusMethodNotAllowed {
|
||||
return nil, fs.ErrorObjectNotFound
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,8 @@ import (
|
||||
"github.com/rclone/rclone/fs"
|
||||
"github.com/rclone/rclone/fstest"
|
||||
"github.com/rclone/rclone/fstest/fstests"
|
||||
"github.com/rclone/rclone/lib/bucket"
|
||||
"github.com/rclone/rclone/lib/pacer"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -129,6 +131,57 @@ func TestClientStopsAfterTenRedirects(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "stopped after 10 redirects")
|
||||
}
|
||||
|
||||
func TestObjectNotFoundMapping(t *testing.T) {
|
||||
ctx, opt, client := SetupS3Test(t)
|
||||
gotHead, gotGet := false, false
|
||||
|
||||
// Return 404 for all requests.
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.Method {
|
||||
case http.MethodHead:
|
||||
gotHead = true
|
||||
case http.MethodGet:
|
||||
gotGet = true
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
opt.Endpoint = server.URL
|
||||
opt.ForcePathStyle = true
|
||||
opt.Region = "us-east-1"
|
||||
opt.AccessKeyID = "id"
|
||||
opt.SecretAccessKey = "secret"
|
||||
c, _, err := s3Connection(ctx, opt, client)
|
||||
require.NoError(t, err)
|
||||
|
||||
f := &Fs{
|
||||
name: "s3test",
|
||||
opt: *opt,
|
||||
ctx: ctx,
|
||||
c: c,
|
||||
pacer: fs.NewPacer(ctx, pacer.NewS3(pacer.MinSleep(minSleep))),
|
||||
cache: bucket.NewCache(),
|
||||
}
|
||||
f.setRoot("bucket")
|
||||
|
||||
// HEAD path: NewObject reads metadata via HeadObject.
|
||||
_, headErr := f.NewObject(ctx, "missing.txt")
|
||||
require.True(t, gotHead, "server should have received a HEAD request")
|
||||
assert.ErrorIs(t, headErr, fs.ErrorObjectNotFound)
|
||||
|
||||
// GET path: Object.Open issues a GetObject.
|
||||
o := &Object{fs: f, remote: "missing.txt"}
|
||||
in, getErr := o.Open(ctx)
|
||||
if in != nil {
|
||||
_ = in.Close()
|
||||
}
|
||||
require.True(t, gotGet, "server should have received a GET request")
|
||||
assert.ErrorIs(t, getErr, fs.ErrorObjectNotFound)
|
||||
|
||||
assert.Equal(t, headErr, getErr, "HeadObject and GetObject should map a 404 to the same error")
|
||||
}
|
||||
|
||||
// TestIntegration runs integration tests against the remote
|
||||
func TestIntegration(t *testing.T) {
|
||||
opt := &fstests.Opt{
|
||||
|
||||
Reference in New Issue
Block a user