Skip to content

Commit

Permalink
Merge pull request #106 from tombuildsstuff/bugfix/table-id-parsing
Browse files Browse the repository at this point in the history
bugfix: more safety when parsing table IDs and table entity IDs, also accept table IDs in legacy and newer formats
  • Loading branch information
manicminer authored Feb 28, 2024
2 parents 9c02373 + 9e9387a commit 22fff23
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 38 deletions.
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

0 comments on commit 22fff23

Please sign in to comment.