Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetObjectAcl of S3 always returns an empty grantee type #1013

Closed
3 tasks done
shogo82148 opened this issue Jan 5, 2021 · 4 comments · Fixed by #1034
Closed
3 tasks done

GetObjectAcl of S3 always returns an empty grantee type #1013

shogo82148 opened this issue Jan 5, 2021 · 4 comments · Fixed by #1034
Labels
bug This issue is a bug.

Comments

@shogo82148
Copy link
Contributor

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug

GetObjectAcl of S3 always returns an empty grantee type.

Version of AWS SDK for Go?

module main

go 1.15

require (
	github.com/aws/aws-sdk-go v1.36.20
	github.com/aws/aws-sdk-go-v2 v0.31.0
	github.com/aws/aws-sdk-go-v2/config v0.4.0
	github.com/aws/aws-sdk-go-v2/service/s3 v0.31.0
	github.com/kr/pretty v0.2.1
)

Version of Go (go version)?

go version go1.15.6 darwin/amd64

To Reproduce

package main

import (
	"context"
	"log"
	"strings"

	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/s3"

	awsv2 "github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	s3v2 "github.com/aws/aws-sdk-go-v2/service/s3"

	"github.com/kr/pretty"
)

func main() {
	const bucket = "shogo82148-test"
	const key = "debug.txt"
	const region = "ap-northeast-1" // the region of shogo82148-test bucket

	sess := session.Must(session.NewSession(&aws.Config{
		Region: aws.String(region),
	}))

	ctx := context.Background()
	v1 := s3.New(sess)
	_, err := v1.PutObjectWithContext(ctx, &s3.PutObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(key),
		Body:   strings.NewReader("foobar"),
	})
	if err != nil {
		log.Fatal(err)
	}

	// AWS SDK v1
	respv1, err := v1.GetObjectAclWithContext(ctx, &s3.GetObjectAclInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(key),
	})
	if err != nil {
		log.Fatal(err)
	}
	pretty.Log(respv1.Grants)

	// AWS SDK v2
	cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(region))
	if err != nil {
		log.Fatal(err)
	}
	v2 := s3v2.NewFromConfig(cfg)
	respv2, err := v2.GetObjectAcl(ctx, &s3v2.GetObjectAclInput{
		Bucket: awsv2.String(bucket),
		Key:    awsv2.String(key),
	})
	if err != nil {
		log.Fatal(err)
	}
	pretty.Log(respv2.Grants)
}

result:

% go run main.go
2021/01/05 16:53:42 []*s3.Grant{
    &s3.Grant{
        _:       struct {}{},
        Grantee: &s3.Grantee{
            _:            struct {}{},
            DisplayName:  &"shogo82148",
            EmailAddress: (*string)(nil),
            ID:           &"51915c5bd71c959533019b53a63318b74bf3f85d6cbe9708d564187d200649c7",
            Type:         &"CanonicalUser",
            URI:          (*string)(nil),
        },
        Permission: &"FULL_CONTROL",
    },
}
2021/01/05 16:53:42 []types.Grant{
    {
        Grantee: &types.Grantee{
            Type:         "",
            DisplayName:  &"shogo82148",
            EmailAddress: (*string)(nil),
            ID:           &"51915c5bd71c959533019b53a63318b74bf3f85d6cbe9708d564187d200649c7",
            URI:          (*string)(nil),
        },
        Permission: "FULL_CONTROL",
    },
}

Expected behavior

I want to get "CanonicalUser" grantee type.

2021/01/05 16:53:42 []types.Grant{
    {
        Grantee: &types.Grantee{
            Type:         "CanonicalUser", // I got ""
            DisplayName:  &"shogo82148",
            EmailAddress: (*string)(nil),
            ID:           &"51915c5bd71c959533019b53a63318b74bf3f85d6cbe9708d564187d200649c7",
            URI:          (*string)(nil),
        },
        Permission: "FULL_CONTROL",
    },
}

AWS SDK v1 returns my expected result, but v2 doesn't.

Additional context

Is types.TypeCanonicaluser intended name? it should be types.TypeCanonicalUser ?

https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/s3/types#Type

@shogo82148 shogo82148 added the bug This issue is a bug. label Jan 5, 2021
jasdel added a commit to jasdel/smithy-go that referenced this issue Jan 6, 2021
Fixes the generated enum value names to not be lowercased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
jasdel added a commit to jasdel/smithy-go that referenced this issue Jan 6, 2021
Fixes the generated enum value names to not be lower cased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
jasdel added a commit to jasdel/smithy-go that referenced this issue Jan 6, 2021
Fixes the generated enum value names to not be lower cased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
jasdel added a commit to jasdel/smithy-go that referenced this issue Jan 6, 2021
Fixes the generated enum value names to not be lower cased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
jasdel added a commit to jasdel/smithy-go that referenced this issue Jan 6, 2021
Fixes the generated enum value names to not be lower cased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
@jasdel
Copy link
Contributor

jasdel commented Jan 6, 2021

Thanks for reporting this issue @shogo82148. The SDK's behavior of not being able to deserialize the Grantee type is definitely a bug in the SDK. This most likely is related to the SDK's handling of XML attributes with namespaces. There was a similar bug in the v1 SDK few years ago, aws/aws-sdk-go#975 and aws/aws-sdk-go#916.

Also thanks for pointing out the unexpected enum name casing. I've corrected the enum casing in #1020.

jasdel added a commit to aws/smithy-go that referenced this issue Jan 7, 2021
Fixes the generated enum value names to not be lower cased when
capitalized.

Related to aws/aws-sdk-go-v2#1013
jasdel added a commit that referenced this issue Jan 7, 2021
…1020)

Fixes API enum parameter generated constant names to have expected casing

Related to aws/smithy-go#252
Related to #1013
@jasdel jasdel added this to the v1.0 General Availability milestone Jan 8, 2021
@JordonPhillips
Copy link
Member

JordonPhillips commented Jan 12, 2021

This does seem to be related to namespaces. For example, the actual XML returned looks something like this:

<Grantee xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="CanonicalUser">
    <ID>foo</ID>
    <DisplayName>bar</DisplayName>
</Grantee>

We try to decode this right around here. The parsed values of the attrs look something like this:

[]xml.Attr{
    {
        Name:  xml.Name{Space:"xmlns", Local:"xsi"},
        Value: "http://www.w3.org/2001/XMLSchema-instance",
    },
    {
        Name:  xml.Name{Space:"http://www.w3.org/2001/XMLSchema-instance", Local:"type"},
        Value: "CanonicalUser",
    },
}

We're checking Name.Local for xsi:type on each of these, but it would never equal that both because the namespace is separate and because it appears fully resolved.

@JordonPhillips
Copy link
Member

And this does appear to be exactly the same issue referenced above

jasdel added a commit to jasdel/smithy that referenced this issue Jan 12, 2021
Adds additional protocol tests for member bound to XML attribute within
a struct that also has an XML namespace.

Related to aws/smithy-go#255, aws/aws-sdk-go-v2#1034, aws/aws-sdk-go-v2#1013
JordonPhillips pushed a commit to smithy-lang/smithy that referenced this issue Jan 13, 2021
Adds additional protocol tests for member bound to XML attribute within
a struct that also has an XML namespace.

Related to aws/smithy-go#255, aws/aws-sdk-go-v2#1034, aws/aws-sdk-go-v2#1013
jasdel added a commit that referenced this issue Jan 14, 2021
Fixes the SDK's generation of unmarshalers for XML attributes in tags with xml namespaces. 

Fixes #1013
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants