fix(route53): handle escape code correctly

.changelog/40154.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_route53_zone: Handle a hosted zone with escape code correctly
+```
+
+```release-note:bug
+resource/aws_route53_record: Handle a record name with escaped escape code correctly
+```

internal/service/route53/clean.go

@@ -4,26 +4,79 @@
 package route53
 
 import (
-	"strconv"
+	"bufio"
+	"fmt"
+	"io"
 	"strings"
+	"unicode"
 
+	"github.com/YakDriver/regexache"
 	"github.com/aws/aws-sdk-go-v2/aws"
 )
 
 func cleanDelegationSetID(id string) string {
 	return strings.TrimPrefix(id, "/delegationset/")
 }
 
-// Route 53 stores certain characters with the octal equivalent in ASCII format.
-// This function converts all of these characters back into the original character.
-// E.g. "*" is stored as "\\052" and "@" as "\\100"
-func cleanRecordName(name string) string {
-	str := name
-	s, err := strconv.Unquote(`"` + str + `"`)
-	if err != nil {
-		return str
+// normalizeNameIntoRoute53APIRepresentation converts an user input into Route53's record name representation.
+// See https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/DomainNameFormat.html#domain-name-format-hosted-zones
+// See https://datatracker.ietf.org/doc/html/rfc4343#section-2.1
+func normalizeNameIntoRoute53APIRepresentation(input string) string {
+	var ret string
+
+	br := bufio.NewReader(strings.NewReader(input))
+
+	for {
+		ch, _, err := br.ReadRune()
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			return ""
+		}
+
+		ch = unicode.ToLower(ch)
+
+		// when backslack is found, check if a beginning of escape code
+		switch {
+		case ch == '\\':
+			esc, err := br.Peek(3)
+			if err == nil {
+				if isAllNumeric(string(esc)) {
+					ret += "\\" + string(esc)
+
+					// advanced 3 bytes
+					_, _ = br.Discard(3)
+					continue
+				}
+			}
+			// treat it as "\"  and carry on
+			ret += "\\"
+		case needRoute53EscapeCode(ch):
+			// convert into escape code
+			ret += fmt.Sprintf("\\%03o", ch)
+		default:
+			ret += string(ch)
+		}
 	}
-	return s
+
+	return ret
+}
+
+func isAllNumeric(s string) bool {
+	for _, r := range s {
+		if r < '0' || r > '9' {
+			return false
+		}
+	}
+	return true
+}
+
+// needRoute53EscapeCode returns true if a given rune needs an escape code for Route53 representation.
+// https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/DomainNameFormat.html#domain-name-format-hosted-zones
+// > If the domain name includes any characters other than a to z, 0 to 9, - (hyphen), or _ (underscore), Route 53 API actions return the characters as escape codes
+func needRoute53EscapeCode(r rune) bool {
+	return !regexache.MustCompile(`[0-9A-Za-z_\-.]`).MatchString(string(r))
 }
 
 // CleanZoneID is used to remove the leading /hostedzone/
@@ -33,7 +86,7 @@ func cleanZoneID(ID string) string {
 
 func normalizeAliasName(alias interface{}) string {
 	output := strings.ToLower(alias.(string))
-	return cleanRecordName(strings.TrimSuffix(output, "."))
+	return normalizeNameIntoRoute53APIRepresentation(strings.TrimSuffix(output, "."))
 }
 
 // normalizeZoneName is used to remove the trailing period
@@ -58,7 +111,9 @@ func normalizeZoneName(v interface{}) string {
 		return str
 	}
 
-	return strings.ToLower(strings.TrimSuffix(str, "."))
+	return normalizeNameIntoRoute53APIRepresentation(
+		strings.ToLower(strings.TrimSuffix(str, ".")),
+	)
 }
 
 func fqdn(name string) string {

internal/service/route53/clean_test.go

@@ -9,23 +9,42 @@ import (
 	"github.com/aws/aws-sdk-go-v2/aws"
 )
 
-func TestCleanRecordName(t *testing.T) {
+func TestNormalizeNameIntoRoute53Representation(t *testing.T) {
 	t.Parallel()
 
 	cases := []struct {
-		Input, Output string
+		UserInput, ExpectedR53Output string
 	}{
+		// Preserve escape code
+		{"a\\000c.example.com", "a\\000c.example.com"},
+		{"a\\056c.example.com", "a\\056c.example.com"}, // with escaped "."
+
+		// Preserve "-" / "_" as-is
+		{"a-b.example.com", "a-b.example.com"},
+		{"_abc.example.com", "_abc.example.com"},
+
+		// no conversion
 		{"www.example.com", "www.example.com"},
-		{"\\052.example.com", "*.example.com"},
-		{"\\100.example.com", "@.example.com"},
-		{"\\043.example.com", "#.example.com"},
-		{"example.com", "example.com"},
+
+		// converted into lower-case
+		{"AbC.example.com", "abc.example.com"},
+
+		// convert into escape code
+		{"*.example.com", "\\052.example.com"},
+		{"!.example.com", "\\041.example.com"},
+		{"a/b.example.com", "a\\057b.example.com"},
+		{"/.example.com", "\\057.example.com"},
+		{"~.example.com", "\\176.example.com"},
 	}
 
 	for _, tc := range cases {
-		actual := cleanRecordName(tc.Input)
-		if actual != tc.Output {
-			t.Fatalf("input: %s\noutput: %s", tc.Input, actual)
+		actual := normalizeNameIntoRoute53APIRepresentation(tc.UserInput)
+
+		if actual != tc.ExpectedR53Output {
+			t.Errorf(
+				"user input: %+q\nexpected: %+q\nr53 output: %+q",
+				tc.UserInput, tc.ExpectedR53Output, actual,
+			)
 		}
 	}
 }

internal/service/route53/exports_test.go

@@ -20,7 +20,6 @@ var (
 	ResourceZoneAssociation             = resourceZoneAssociation
 
 	CleanDelegationSetID                        = cleanDelegationSetID
-	CleanRecordName                             = cleanRecordName
 	CleanZoneID                                 = cleanZoneID
 	ExpandRecordName                            = expandRecordName
 	FindCIDRCollectionByID                      = findCIDRCollectionByID
@@ -43,6 +42,7 @@ var (
 	ServeSignatureNotSigning                    = serveSignatureNotSigning
 	ServeSignatureSigning                       = serveSignatureSigning
 	WaitChangeInsync                            = waitChangeInsync
+	NormalizeNameIntoRoute53APIRepresentation   = normalizeNameIntoRoute53APIRepresentation
 )
 
 type Route53TrafficPolicyDoc = route53TrafficPolicyDoc

internal/service/route53/record.go

@@ -791,7 +791,7 @@ func findResourceRecordSetByFourPartKey(ctx context.Context, conn *route53.Clien
 	}
 
 	name := expandRecordName(recordName, aws.ToString(zone.HostedZone.Name))
-	recordName = fqdn(strings.ToLower(name))
+	recordName = normalizeNameIntoRoute53APIRepresentation(fqdn(strings.ToLower(name)))
 	rrType := awstypes.RRType(strings.ToUpper(recordType))
 	input := &route53.ListResourceRecordSetsInput{
 		HostedZoneId:    aws.String(zoneID),
@@ -804,7 +804,7 @@ func findResourceRecordSetByFourPartKey(ctx context.Context, conn *route53.Clien
 		input.MaxItems = aws.Int32(100)
 	}
 	output, err := findResourceRecordSet(ctx, conn, input, resourceRecordsFor(recordName, rrType), func(v *awstypes.ResourceRecordSet) bool {
-		if recordName != strings.ToLower(cleanRecordName(aws.ToString(v.Name))) {
+		if recordName != strings.ToLower(aws.ToString(v.Name)) {
 			return false
 		}
 		if recordType != strings.ToUpper(string(v.Type)) {
@@ -869,7 +869,7 @@ func findResourceRecordSets(ctx context.Context, conn *route53.Client, input *ro
 func resourceRecordsFor(recordName string, recordType awstypes.RRType) tfslices.Predicate[*route53.ListResourceRecordSetsOutput] {
 	return func(page *route53.ListResourceRecordSetsOutput) bool {
 		if page.IsTruncated {
-			if strings.ToLower(cleanRecordName(aws.ToString(page.NextRecordName))) != recordName {
+			if strings.ToLower(aws.ToString(page.NextRecordName)) != recordName {
 				return false
 			}
 

internal/service/route53/record_test.go

@@ -76,6 +76,96 @@ func TestAccRoute53Record_basic(t *testing.T) {
 	})
 }
 
+func TestAccRoute53Record_Create_escape_code_slash(t *testing.T) {
+	ctx := acctest.Context(t)
+	var v awstypes.ResourceRecordSet
+	resourceName := "aws_route53_record.test"
+
+	zoneName := "0/24." + acctest.RandomDomain()
+	recordName := "0"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.Route53ServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckRecordDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRecordConfig_basic(zoneName.String(), recordName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists(ctx, resourceName, &v),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccRoute53Record_Create_escape_code_space(t *testing.T) {
+	ctx := acctest.Context(t)
+	var v awstypes.ResourceRecordSet
+	resourceName := "aws_route53_record.test"
+
+	zoneName := "a\\040b." + acctest.RandomDomain()
+	recordName := "0\\040to\\0401"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.Route53ServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckRecordDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRecordConfig_basic(zoneName.String(), recordName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists(ctx, resourceName, &v),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccRoute53Record_Create_escape_code_just_space(t *testing.T) {
+	ctx := acctest.Context(t)
+	var v awstypes.ResourceRecordSet
+	resourceName := "aws_route53_record.test"
+
+	// zone name always needs an escape code if any
+	zoneName := "a\\040b." + acctest.RandomDomain()
+
+	// as for record name, r53 API can accept a space as is but will send the escaped version of it back
+	recordName := "0 to 1"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.Route53ServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckRecordDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRecordConfig_basic(zoneName.String(), recordName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists(ctx, resourceName, &v),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccRoute53Record_disappears(t *testing.T) {
 	ctx := acctest.Context(t)
 	var v awstypes.ResourceRecordSet
@@ -1682,7 +1772,7 @@ func testAccCheckRecordDoesNotExist(ctx context.Context, zoneResourceName string
 
 		found := false
 		for _, rec := range resp.ResourceRecordSets {
-			recName := tfroute53.CleanRecordName(*rec.Name)
+			recName := tfroute53.NormalizeNameIntoRoute53APIRepresentation(*rec.Name)
 			if tfroute53.FQDN(strings.ToLower(recName)) == tfroute53.FQDN(strings.ToLower(en)) && rec.Type == awstypes.RRType(recordType) {
 				found = true
 				break

internal/service/route53/zone_test.go

@@ -70,6 +70,64 @@ func TestAccRoute53Zone_basic(t *testing.T) {
 	})
 }
 
+func TestAccRoute53Zone_Create_escape_code_slash(t *testing.T) {
+	ctx := acctest.Context(t)
+	var zone route53.GetHostedZoneOutput
+	resourceName := "aws_route53_zone.test"
+	zoneName := "0/24." + acctest.RandomDomainName()
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.Route53ServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckZoneDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccZoneConfig_basic(zoneName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckZoneExists(ctx, resourceName, &zone),
+				),
+			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{names.AttrForceDestroy},
+			},
+		},
+	})
+}
+
+func TestAccRoute53Zone_Create_escape_code_space(t *testing.T) {
+	ctx := acctest.Context(t)
+	var zone route53.GetHostedZoneOutput
+	resourceName := "aws_route53_zone.test"
+
+	// double-escape is required for templating the tf resource
+	zoneName := "a\\\\040b." + acctest.RandomDomainName()
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.Route53ServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckZoneDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccZoneConfig_basic(zoneName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckZoneExists(ctx, resourceName, &zone),
+				),
+			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{names.AttrForceDestroy},
+			},
+		},
+	})
+}
+
 func TestAccRoute53Zone_disappears(t *testing.T) {
 	ctx := acctest.Context(t)
 	var zone route53.GetHostedZoneOutput