[s3 gateway] Handle all path characters in SigV2 and add Nessie S3-ish test #2464
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arielshaqed
force-pushed
the
bugfix/2445-nonascii-sigv2
branch
2 times, most recently
from
a7588b2
to
8f2794f
Compare
arielshaqed
changed the title
arielshaqed
force-pushed
the
bugfix/2445-nonascii-sigv2
branch
6 times, most recently
from
f7c2b54
to
bac9681
Compare
arielshaqed
force-pushed
the
bugfix/2445-nonascii-sigv2
branch
2 times, most recently
from
c45837f
to
1b9109c
Compare
arielshaqed
force-pushed
the
bugfix/2445-nonascii-sigv2
branch
from
02b2315
to
6f9f5dc
Compare
guy-har
approved these changes
Sep 14, 2021
pkg/gateway/sig/v4.go
Outdated
| @@ -277,6 +277,7 @@ func (ctx *verificationCtx) getAmzDate() (string, error) { | |||
| // parse signature date | |||
| sigTS, err := time.Parse(v4shortTimeFormat, ctx.AuthValue.Date) | |||
| if err != nil { | |||
| fmt.Printf("[DEBUG] failed to parse auth-date %s by %s: %s\n", ctx.AuthValue.Date, v4shortTimeFormat, err) | |||
There was a problem hiding this comment.
😢 sorry, thanks!
nessie/s3_gateway_test.go
Outdated
| download, err := client.GetObject( | ||
| ctx, repo, o.Path, minio.GetObjectOptions{}) | ||
| if err != nil { | ||
| t.Error(fmt.Sprintf("minio.Client.GetObject(%s): %s", o.Path, err)) |
There was a problem hiding this comment.
You're right, that was silly of me.
nessie/s3_gateway_test.go
Outdated
| Secure: false, | ||
| }) | ||
| if err != nil { | ||
| t.Error(fmt.Sprintf("minio.New: %s", err)) |
There was a problem hiding this comment.
why aren't you returning in case of error? will be hard to continue without a client
There was a problem hiding this comment.
Good catch! Dying fatally outside the goroutine when this fails.
arielshaqed
commented
Sep 14, 2021
nessie/s3_gateway_test.go
Outdated
| download, err := client.GetObject( | ||
| ctx, repo, o.Path, minio.GetObjectOptions{}) | ||
| if err != nil { | ||
| t.Error(fmt.Sprintf("minio.Client.GetObject(%s): %s", o.Path, err)) |
There was a problem hiding this comment.
You're right, that was silly of me.
nessie/s3_gateway_test.go
Outdated
| Secure: false, | ||
| }) | ||
| if err != nil { | ||
| t.Error(fmt.Sprintf("minio.New: %s", err)) |
There was a problem hiding this comment.
Good catch! Dying fatally outside the goroutine when this fails.
Use the minIO client to test S3 uploads and downloads with both V2 and V4 signatures. (Unlike the AWS S3 client, the minIO client supports signing using SigV2.)
(Timed out on GCP)
Sign using minio, verify using gateway code.
Also increase parallelism somewhat.
arielshaqed
force-pushed
the
bugfix/2445-nonascii-sigv2
branch
from
f54ba60
to
a0ed39f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use correctly-coded path when checking sigV2 signatures in the S3 gateway.
Use the minIO client to test S3 uploads and downloads with both V2 and V4
signatures. (Unlike the AWS S3 client, the minIO client supports signing
using SigV2.)