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

bugfix: more safety when parsing table IDs and table entity IDs, also accept table IDs in legacy and newer formats #106

Merged
merged 4 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ toolchain go1.21.3
require (
github.com/google/uuid v1.4.0
github.com/hashicorp/go-azure-helpers v0.66.2
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240223.1153421
github.com/hashicorp/go-azure-sdk/sdk v0.20240223.1153421
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240227.1172434
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1172434
github.com/stretchr/testify v1.8.4
)

Expand Down
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,20 @@ github.com/hashicorp/go-azure-sdk/resource-manager v0.20240125.1111756 h1:foZtDG
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240125.1111756/go.mod h1:yWRLLjG7PVThIY1NLeLNhp0VtTewOCFrIVIZl/LgPBc=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240223.1153421 h1:RrPR6RGkkvfWoP+UkmzWNQwoIVsFO/Oj+8lQqQw3880=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240223.1153421/go.mod h1:gm/5ZkCKtVTV1sSHyMql5a8tB0Z5NBXjF0MGB7Q8chg=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240227.1130345 h1:h+GSWVxpVzTH0kD+SyRskBsJuKBVqJVCGItMmLsO50I=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240227.1130345/go.mod h1:G2gxM62VIehZbxVxxmkOSQxngB/ecH9QDO+sgDSTnfo=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240227.1172434 h1:UNp65kdO4noJDrEe99Od5NfXSnjrrTs0dEyw2MM8jK0=
github.com/hashicorp/go-azure-sdk/resource-manager v0.20240227.1172434/go.mod h1:8Pmp8Bg+FDUjkbuuiLLqysKrKUu5dOIf1dX3KFKNSU8=
github.com/hashicorp/go-azure-sdk/sdk v0.20240219.1162257-0.20240220115734-eeb1a5d96f9a h1:9Qg8M1Yp3WGmJw0FyAeg/4VXmp0m+6kfSJ//etdAfLk=
github.com/hashicorp/go-azure-sdk/sdk v0.20240219.1162257-0.20240220115734-eeb1a5d96f9a/go.mod h1:IKIPyL+hfFWBHABKT0NOWlIEzlusiUBG0SxIfaiv278=
github.com/hashicorp/go-azure-sdk/sdk v0.20240223.1153421 h1:unXuyut6yDlY9kaoworHj2f1gd/c+mSf8qh3WkMyB1Q=
github.com/hashicorp/go-azure-sdk/sdk v0.20240223.1153421/go.mod h1:IKIPyL+hfFWBHABKT0NOWlIEzlusiUBG0SxIfaiv278=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1130345 h1:iIfqojPBH9SskoKqcA1bZGbGQcEWyjx+2+0PHwQafgc=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1130345/go.mod h1:IKIPyL+hfFWBHABKT0NOWlIEzlusiUBG0SxIfaiv278=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1172434 h1:IX9e6DRUYl+1skjZPpfdhnEV3cx8dNX1qECYSVcD288=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1172434/go.mod h1:IKIPyL+hfFWBHABKT0NOWlIEzlusiUBG0SxIfaiv278=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1200200 h1:o0D7vswmP3fnm9slJ8YcsQJq7QogeMAjq5yl3a+kbDo=
github.com/hashicorp/go-azure-sdk/sdk v0.20240227.1200200/go.mod h1:IKIPyL+hfFWBHABKT0NOWlIEzlusiUBG0SxIfaiv278=
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 h1:1/D3zfFHttUKaCaGKZ/dR2roBXv0vKbSCnssIldfQdI=
Expand Down
2 changes: 1 addition & 1 deletion storage/2020-08-04/blob/accounts/get_service_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c Client) GetServiceProperties(ctx context.Context, accountName string) (r
}

opts := client.RequestOptions{
ContentType: "text/xml",
ContentType: "application/xml",
ExpectedStatusCodes: []int{
http.StatusOK,
},
Expand Down
2 changes: 1 addition & 1 deletion storage/2020-08-04/blob/accounts/set_service_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (c Client) SetServiceProperties(ctx context.Context, accountName string, in
}

opts := client.RequestOptions{
ContentType: "text/xml",
ContentType: "application/xml",
ExpectedStatusCodes: []int{
http.StatusAccepted,
},
Expand Down
21 changes: 13 additions & 8 deletions storage/2020-08-04/table/entities/resource_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (b EntityId) String() string {

// ParseEntityID parses `input` into a Entity ID using a known `domainSuffix`
func ParseEntityID(input, domainSuffix string) (*EntityId, error) {
// example: https://foo.table.core.windows.net/Bar1(PartitionKey='partition1',RowKey='row1')
// example: https://foo.table.core.windows.net/bar(PartitionKey='partition1',RowKey='row1')
if input == "" {
return nil, fmt.Errorf("`input` was empty")
}
Expand All @@ -79,23 +79,28 @@ func ParseEntityID(input, domainSuffix string) (*EntityId, error) {

// Tables and Table Entities are similar with table being `table1` and entities
// being `table1(PartitionKey='samplepartition',RowKey='samplerow')` so we need to validate this is a table
key := strings.TrimPrefix(uri.Path, "/")
if !strings.Contains(key, "(") || !strings.HasSuffix(key, ")") {
return nil, fmt.Errorf("expected the path to be an entity name but got a table name %q", key)
slug := strings.TrimPrefix(uri.Path, "/")
if strings.HasPrefix(slug, "Tables('") && strings.HasSuffix(slug, "')") {
// Ensure we do not parse a Table ID in the format: https://foo.table.core.windows.net/Table('foo')
return nil, fmt.Errorf("expected the path to be an entity name but got a table name: %q", slug)
} else if !strings.Contains(slug, "(") || !strings.HasSuffix(slug, ")") {
// Ensure we do not try to parse a bare table name
return nil, fmt.Errorf("expected the path to be an entity name but got an invalid format, possibly a table name: %q", slug)
}

indexOfFirstBracket := strings.Index(key, "(")
tableName := key[0:indexOfFirstBracket]
componentString := key[indexOfFirstBracket:]
indexOfFirstBracket := strings.Index(slug, "(")
tableName := slug[0:indexOfFirstBracket]
componentString := slug[indexOfFirstBracket:]
componentString = strings.TrimPrefix(componentString, "(")
componentString = strings.TrimSuffix(componentString, ")")
components := strings.Split(componentString, ",")
if len(components) != 2 {
return nil, fmt.Errorf("expected the path to be an entity name but got %q", key)
return nil, fmt.Errorf("expected the path to be an entity name but got %q", slug)
}

partitionKey := parseValueFromKey(components[0], "PartitionKey")
rowKey := parseValueFromKey(components[1], "RowKey")

return &EntityId{
AccountId: *account,
TableName: tableName,
Expand Down
25 changes: 20 additions & 5 deletions storage/2020-08-04/table/tables/resource_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (b TableId) String() string {

// ParseTableID parses `input` into a Table ID using a known `domainSuffix`
func ParseTableID(input, domainSuffix string) (*TableId, error) {
// example: https://foo.table.core.windows.net/Bar
// example: https://foo.table.core.windows.net/Table('bar')
if input == "" {
return nil, fmt.Errorf("`input` was empty")
}
Expand All @@ -73,12 +73,27 @@ func ParseTableID(input, domainSuffix string) (*TableId, error) {
return nil, fmt.Errorf("expected the path to contain 1 segment but got %d", len(segments))
}

// Tables and Table Entities are similar with table being `table1` and entities
// being `table1(PartitionKey='samplepartition',RowKey='samplerow')` so we need to validate this is a table
tableName := strings.TrimPrefix(uri.Path, "/")
if strings.Contains(tableName, "(") || strings.Contains(tableName, ")") {
// Tables and Table Entities are similar however Tables use a reserved namespace, for example:
// Table('tableName')
// whereas Entities begin with the actual table name, for example:
// tableName(PartitionKey='samplepartition',RowKey='samplerow')
// However, there was a period of time when Table IDs did not use the reserved namespace, so we attempt to parse
// both forms for maximum compatibility.
var tableName string
slug := strings.TrimPrefix(uri.Path, "/")
if strings.HasPrefix(slug, "Tables('") && strings.HasSuffix(slug, "')") {
// Ensure both prefix and suffix are present before trimming them out
tableName = strings.TrimSuffix(strings.TrimPrefix(slug, "Tables('"), "')")
} else if !strings.Contains(slug, "(") && !strings.HasSuffix(slug, ")") {
// Also accept a bare table name
tableName = slug
} else {
return nil, fmt.Errorf("expected the path to a table name and not an entity name but got %q", tableName)
}
if tableName == "" {
return nil, fmt.Errorf("expected the path to a table name but the path was empty")
}

return &TableId{
AccountId: *account,
TableName: tableName,
Expand Down
2 changes: 1 addition & 1 deletion storage/2023-11-03/blob/accounts/get_service_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c Client) GetServiceProperties(ctx context.Context, accountName string) (r
}

opts := client.RequestOptions{
ContentType: "text/xml",
ContentType: "application/xml; charset=utf-8",
ExpectedStatusCodes: []int{
http.StatusOK,
},
Expand Down
2 changes: 1 addition & 1 deletion storage/2023-11-03/blob/accounts/set_service_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (c Client) SetServiceProperties(ctx context.Context, accountName string, in
}

opts := client.RequestOptions{
ContentType: "text/xml",
ContentType: "application/xml; charset=utf-8",
ExpectedStatusCodes: []int{
http.StatusAccepted,
},
Expand Down
21 changes: 13 additions & 8 deletions storage/2023-11-03/table/entities/resource_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (b EntityId) String() string {

// ParseEntityID parses `input` into a Entity ID using a known `domainSuffix`
func ParseEntityID(input, domainSuffix string) (*EntityId, error) {
// example: https://foo.table.core.windows.net/Bar1(PartitionKey='partition1',RowKey='row1')
// example: https://foo.table.core.windows.net/bar(PartitionKey='partition1',RowKey='row1')
if input == "" {
return nil, fmt.Errorf("`input` was empty")
}
Expand All @@ -79,23 +79,28 @@ func ParseEntityID(input, domainSuffix string) (*EntityId, error) {

// Tables and Table Entities are similar with table being `table1` and entities
// being `table1(PartitionKey='samplepartition',RowKey='samplerow')` so we need to validate this is a table
key := strings.TrimPrefix(uri.Path, "/")
if !strings.Contains(key, "(") || !strings.HasSuffix(key, ")") {
return nil, fmt.Errorf("expected the path to be an entity name but got a table name %q", key)
slug := strings.TrimPrefix(uri.Path, "/")
if strings.HasPrefix(slug, "Tables('") && strings.HasSuffix(slug, "')") {
// Ensure we do not parse a Table ID in the format: https://foo.table.core.windows.net/Table('foo')
return nil, fmt.Errorf("expected the path to be an entity name but got a table name: %q", slug)
} else if !strings.Contains(slug, "(") || !strings.HasSuffix(slug, ")") {
// Ensure we do not try to parse a bare table name
return nil, fmt.Errorf("expected the path to be an entity name but got an invalid format, possibly a table name: %q", slug)
}

indexOfFirstBracket := strings.Index(key, "(")
tableName := key[0:indexOfFirstBracket]
componentString := key[indexOfFirstBracket:]
indexOfFirstBracket := strings.Index(slug, "(")
tableName := slug[0:indexOfFirstBracket]
componentString := slug[indexOfFirstBracket:]
componentString = strings.TrimPrefix(componentString, "(")
componentString = strings.TrimSuffix(componentString, ")")
components := strings.Split(componentString, ",")
if len(components) != 2 {
return nil, fmt.Errorf("expected the path to be an entity name but got %q", key)
return nil, fmt.Errorf("expected the path to be an entity name but got %q", slug)
}

partitionKey := parseValueFromKey(components[0], "PartitionKey")
rowKey := parseValueFromKey(components[1], "RowKey")

return &EntityId{
AccountId: *account,
TableName: tableName,
Expand Down
27 changes: 21 additions & 6 deletions storage/2023-11-03/table/tables/resource_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewTableID(accountId accounts.AccountId, tableName string) TableId {
}

func (b TableId) ID() string {
return fmt.Sprintf("%s/%s", b.AccountId.ID(), b.TableName)
return fmt.Sprintf("%s/Tables('%s')", b.AccountId.ID(), b.TableName)
}

func (b TableId) String() string {
Expand All @@ -48,7 +48,7 @@ func (b TableId) String() string {

// ParseTableID parses `input` into a Table ID using a known `domainSuffix`
func ParseTableID(input, domainSuffix string) (*TableId, error) {
// example: https://foo.table.core.windows.net/Bar
// example: https://foo.table.core.windows.net/Table('bar')
if input == "" {
return nil, fmt.Errorf("`input` was empty")
}
Expand All @@ -73,12 +73,27 @@ func ParseTableID(input, domainSuffix string) (*TableId, error) {
return nil, fmt.Errorf("expected the path to contain 1 segment but got %d", len(segments))
}

// Tables and Table Entities are similar with table being `table1` and entities
// being `table1(PartitionKey='samplepartition',RowKey='samplerow')` so we need to validate this is a table
tableName := strings.TrimPrefix(uri.Path, "/")
if strings.Contains(tableName, "(") || strings.Contains(tableName, ")") {
// Tables and Table Entities are similar however Tables use a reserved namespace, for example:
// Table('tableName')
// whereas Entities begin with the actual table name, for example:
// tableName(PartitionKey='samplepartition',RowKey='samplerow')
// However, there was a period of time when Table IDs did not use the reserved namespace, so we attempt to parse
// both forms for maximum compatibility.
var tableName string
slug := strings.TrimPrefix(uri.Path, "/")
if strings.HasPrefix(slug, "Tables('") && strings.HasSuffix(slug, "')") {
// Ensure both prefix and suffix are present before trimming them out
tableName = strings.TrimSuffix(strings.TrimPrefix(slug, "Tables('"), "')")
} else if !strings.Contains(slug, "(") && !strings.HasSuffix(slug, ")") {
// Also accept a bare table name
tableName = slug
} else {
return nil, fmt.Errorf("expected the path to a table name and not an entity name but got %q", tableName)
}
if tableName == "" {
return nil, fmt.Errorf("expected the path to a table name but the path was empty")
}

return &TableId{
AccountId: *account,
TableName: tableName,
Expand Down
38 changes: 33 additions & 5 deletions storage/2023-11-03/table/tables/resource_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,34 @@ func TestGetResourceManagerResourceID(t *testing.T) {
}

func TestParseTableIDStandard(t *testing.T) {
input := "https://example1.table.core.windows.net/Tables('table1')"
expected := TableId{
AccountId: accounts.AccountId{
AccountName: "example1",
SubDomainType: accounts.TableSubDomainType,
DomainSuffix: "core.windows.net",
},
TableName: "table1",
}
actual, err := ParseTableID(input, "core.windows.net")
if err != nil {
t.Fatalf(err.Error())
}
if actual.AccountId.AccountName != expected.AccountId.AccountName {
t.Fatalf("expected AccountName to be %q but got %q", expected.AccountId.AccountName, actual.AccountId.AccountName)
}
if actual.AccountId.SubDomainType != expected.AccountId.SubDomainType {
t.Fatalf("expected SubDomainType to be %q but got %q", expected.AccountId.SubDomainType, actual.AccountId.SubDomainType)
}
if actual.AccountId.DomainSuffix != expected.AccountId.DomainSuffix {
t.Fatalf("expected DomainSuffix to be %q but got %q", expected.AccountId.DomainSuffix, actual.AccountId.DomainSuffix)
}
if actual.TableName != expected.TableName {
t.Fatalf("expected TableName to be %q but got %q", expected.TableName, actual.TableName)
}
}

func TestParseTableIDLegacy(t *testing.T) {
input := "https://example1.table.core.windows.net/table1"
expected := TableId{
AccountId: accounts.AccountId{
Expand Down Expand Up @@ -44,7 +72,7 @@ func TestParseTableIDStandard(t *testing.T) {
}

func TestParseTableIDInADNSZone(t *testing.T) {
input := "https://example1.zone1.table.storage.azure.net/table1"
input := "https://example1.zone1.table.storage.azure.net/Tables('table1')"
expected := TableId{
AccountId: accounts.AccountId{
AccountName: "example1",
Expand Down Expand Up @@ -76,7 +104,7 @@ func TestParseTableIDInADNSZone(t *testing.T) {
}

func TestParseTableIDInAnEdgeZone(t *testing.T) {
input := "https://example1.table.zone1.edgestorage.azure.net/table1"
input := "https://example1.table.zone1.edgestorage.azure.net/Tables('table1')"
expected := TableId{
AccountId: accounts.AccountId{
AccountName: "example1",
Expand Down Expand Up @@ -121,7 +149,7 @@ func TestFormatTableIDStandard(t *testing.T) {
},
TableName: "table1",
}.ID()
expected := "https://example1.table.core.windows.net/table1"
expected := "https://example1.table.core.windows.net/Tables('table1')"
if actual != expected {
t.Fatalf("expected %q but got %q", expected, actual)
}
Expand All @@ -138,7 +166,7 @@ func TestFormatTableIDInDNSZone(t *testing.T) {
},
TableName: "table1",
}.ID()
expected := "https://example1.zone2.table.storage.azure.net/table1"
expected := "https://example1.zone2.table.storage.azure.net/Tables('table1')"
if actual != expected {
t.Fatalf("expected %q but got %q", expected, actual)
}
Expand All @@ -155,7 +183,7 @@ func TestFormatTableIDInEdgeZone(t *testing.T) {
},
TableName: "table1",
}.ID()
expected := "https://example1.table.zone2.edgestorage.azure.net/table1"
expected := "https://example1.table.zone2.edgestorage.azure.net/Tables('table1')"
if actual != expected {
t.Fatalf("expected %q but got %q", expected, actual)
}
Expand Down
Loading