From f99f2cfa11b94ccfb5017535e86fd5c5330e54dd Mon Sep 17 00:00:00 2001 From: Wu Tingfeng Date: Thu, 15 Jun 2023 11:08:48 +0800 Subject: [PATCH 1/2] idna: *labelIter.next() to skip last label only if it is empty and verifyDNSLength is false. The existing implementation always skips the empty last label regardless of what verifyDNSLength is set to, which causes pure-ASCII domains ending with a single empty root label to be wrongly accepted when verifyDNSLength is true. This behavior is described in the Unicode Technical Standard 46 at https://unicode.org/reports/tr46/\#ToASCII Fixes https://github.com/golang/go/issues/47182 --- internal/export/idna/idna10.0.0.go | 18 ++++++++++-------- internal/export/idna/idna10.0.0_test.go | 4 +++- internal/export/idna/idna9.0.0.go | 18 ++++++++++-------- internal/export/idna/idna9.0.0_test.go | 4 +++- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/internal/export/idna/idna10.0.0.go b/internal/export/idna/idna10.0.0.go index 0e7571d1..5cc75e68 100644 --- a/internal/export/idna/idna10.0.0.go +++ b/internal/export/idna/idna10.0.0.go @@ -351,7 +351,7 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { if err == nil && p.verifyDNSLength && s == "" { err = &labelError{s, "A4"} } - labels := labelIter{orig: s} + labels := labelIter{orig: s, verifyDNSLength: p.verifyDNSLength} for ; !labels.done(); labels.next() { label := labels.label() if label == "" { @@ -538,11 +538,12 @@ func validateAndMap(p *Profile, s string) (vm string, bidi bool, err error) { // A labelIter allows iterating over domain name labels. type labelIter struct { - orig string - slice []string - curStart int - curEnd int - i int + orig string + slice []string + curStart int + curEnd int + i int + verifyDNSLength bool } func (l *labelIter) reset() { @@ -574,7 +575,8 @@ func (l *labelIter) label() string { return l.orig[l.curStart:l.curEnd] } -// next sets the value to the next label. It skips the last label if it is empty. +// next sets the value to the next label. It skips the last label if it is empty +// and l.verifyDNSLength is false. func (l *labelIter) next() { l.i++ if l.slice != nil { @@ -583,7 +585,7 @@ func (l *labelIter) next() { } } else { l.curStart = l.curEnd + 1 - if l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { + if !l.verifyDNSLength && l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { l.curStart = len(l.orig) } } diff --git a/internal/export/idna/idna10.0.0_test.go b/internal/export/idna/idna10.0.0_test.go index c3365bc6..657f59f6 100644 --- a/internal/export/idna/idna10.0.0_test.go +++ b/internal/export/idna/idna10.0.0_test.go @@ -67,7 +67,8 @@ func TestLabelErrors(t *testing.T) { {lengthA, ".b", ".b", "A4"}, {lengthA, "\u3002b", ".b", "A4"}, {lengthA, "..b", "..b", "A4"}, - {lengthA, "b..", "b..", ""}, + {lengthA, "b..", "b..", "A4"}, + {lengthA, "ƀ..", "xn--lha..", "A4"}, // Sharpened Bidi rules for Unicode 10.0.0. Apply for ALL labels in ANY // of the labels is RTL. @@ -81,6 +82,7 @@ func TestLabelErrors(t *testing.T) { {resolve, "\u3002b", ".b", ""}, {resolve, "..b", "..b", ""}, {resolve, "b..", "b..", ""}, + {resolve, "ƀ..", "xn--lha..", ""}, {resolve, "\xed", "", "P1"}, // Raw punycode diff --git a/internal/export/idna/idna9.0.0.go b/internal/export/idna/idna9.0.0.go index f217b1a4..698471d7 100644 --- a/internal/export/idna/idna9.0.0.go +++ b/internal/export/idna/idna9.0.0.go @@ -351,7 +351,7 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { if err == nil && p.verifyDNSLength && s == "" { err = &labelError{s, "A4"} } - labels := labelIter{orig: s} + labels := labelIter{orig: s, verifyDNSLength: p.verifyDNSLength} for ; !labels.done(); labels.next() { label := labels.label() if label == "" { @@ -500,11 +500,12 @@ func validateAndMap(p *Profile, s string) (string, error) { // A labelIter allows iterating over domain name labels. type labelIter struct { - orig string - slice []string - curStart int - curEnd int - i int + orig string + slice []string + curStart int + curEnd int + i int + verifyDNSLength bool } func (l *labelIter) reset() { @@ -536,7 +537,8 @@ func (l *labelIter) label() string { return l.orig[l.curStart:l.curEnd] } -// next sets the value to the next label. It skips the last label if it is empty. +// next sets the value to the next label. It skips the last label if it is empty +// and l.verifyDNSLength is false. func (l *labelIter) next() { l.i++ if l.slice != nil { @@ -545,7 +547,7 @@ func (l *labelIter) next() { } } else { l.curStart = l.curEnd + 1 - if l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { + if !l.verifyDNSLength && l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { l.curStart = len(l.orig) } } diff --git a/internal/export/idna/idna9.0.0_test.go b/internal/export/idna/idna9.0.0_test.go index 91a1e90d..26f36225 100644 --- a/internal/export/idna/idna9.0.0_test.go +++ b/internal/export/idna/idna9.0.0_test.go @@ -71,13 +71,15 @@ func TestLabelErrors(t *testing.T) { {lengthA, ".b", "b", ""}, {lengthA, "\u3002b", "b", ""}, {lengthA, "..b", "b", ""}, - {lengthA, "b..", "b..", ""}, + {lengthA, "b..", "b..", "A4"}, + {lengthA, "ƀ..", "xn--lha..", "A4"}, {resolve, "a..b", "a..b", ""}, {resolve, ".b", "b", ""}, {resolve, "\u3002b", "b", ""}, {resolve, "..b", "b", ""}, {resolve, "b..", "b..", ""}, + {resolve, "ƀ..", "xn--lha..", ""}, {resolve, "\xed", "", "P1"}, // Raw punycode From 8e763eb0b9cb67fb391e6e62c4150815bd9d6562 Mon Sep 17 00:00:00 2001 From: Wu Tingfeng Date: Fri, 23 Jun 2023 05:31:06 +0800 Subject: [PATCH 2/2] idna: *Profile.process to throw labelError if verifyDNSLength = true and s ends with trailing dot. From UTS46, no trailing dots are allowed when ToASCII is called with verifyDNSLength = true. This negates the need for an additional l.verifyDNSLength flag in the previous commit. See https://www.unicode.org/L2/L2021/21170-utc169-properties-recs.pdf under section F2 Fixes https://github.com/golang/go/issues/47182 --- internal/export/idna/idna10.0.0.go | 27 ++++++++++--------------- internal/export/idna/idna10.0.0_test.go | 8 ++++++++ internal/export/idna/idna9.0.0.go | 27 ++++++++++--------------- internal/export/idna/idna9.0.0_test.go | 8 ++++++++ 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/internal/export/idna/idna10.0.0.go b/internal/export/idna/idna10.0.0.go index 5cc75e68..01bdbb16 100644 --- a/internal/export/idna/idna10.0.0.go +++ b/internal/export/idna/idna10.0.0.go @@ -348,10 +348,10 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { // TODO: allow for a quick check of the tables data. // It seems like we should only create this error on ToASCII, but the // UTS 46 conformance tests suggests we should always check this. - if err == nil && p.verifyDNSLength && s == "" { + if err == nil && p.verifyDNSLength && (s == "" || s[len(s)-1] == '.') { err = &labelError{s, "A4"} } - labels := labelIter{orig: s, verifyDNSLength: p.verifyDNSLength} + labels := labelIter{orig: s} for ; !labels.done(); labels.next() { label := labels.label() if label == "" { @@ -413,12 +413,9 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { } s = labels.result() if toASCII && p.verifyDNSLength && err == nil { - // Compute the length of the domain name minus the root label and its dot. + // Compute the length of the domain name. n := len(s) - if n > 0 && s[n-1] == '.' { - n-- - } - if len(s) < 1 || n > 253 { + if n < 1 || n > 253 { err = &labelError{s, "A4"} } } @@ -538,12 +535,11 @@ func validateAndMap(p *Profile, s string) (vm string, bidi bool, err error) { // A labelIter allows iterating over domain name labels. type labelIter struct { - orig string - slice []string - curStart int - curEnd int - i int - verifyDNSLength bool + orig string + slice []string + curStart int + curEnd int + i int } func (l *labelIter) reset() { @@ -575,8 +571,7 @@ func (l *labelIter) label() string { return l.orig[l.curStart:l.curEnd] } -// next sets the value to the next label. It skips the last label if it is empty -// and l.verifyDNSLength is false. +// next sets the value to the next label. It skips the last label if it is empty. func (l *labelIter) next() { l.i++ if l.slice != nil { @@ -585,7 +580,7 @@ func (l *labelIter) next() { } } else { l.curStart = l.curEnd + 1 - if !l.verifyDNSLength && l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { + if l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { l.curStart = len(l.orig) } } diff --git a/internal/export/idna/idna10.0.0_test.go b/internal/export/idna/idna10.0.0_test.go index 657f59f6..ff9218dd 100644 --- a/internal/export/idna/idna10.0.0_test.go +++ b/internal/export/idna/idna10.0.0_test.go @@ -67,8 +67,12 @@ func TestLabelErrors(t *testing.T) { {lengthA, ".b", ".b", "A4"}, {lengthA, "\u3002b", ".b", "A4"}, {lengthA, "..b", "..b", "A4"}, + {lengthA, "b.", "b.", "A4"}, + {lengthA, "ƀ.", "xn--lha.", "A4"}, {lengthA, "b..", "b..", "A4"}, {lengthA, "ƀ..", "xn--lha..", "A4"}, + {lengthA, "b...", "b...", "A4"}, + {lengthA, "ƀ...", "xn--lha...", "A4"}, // Sharpened Bidi rules for Unicode 10.0.0. Apply for ALL labels in ANY // of the labels is RTL. @@ -81,8 +85,12 @@ func TestLabelErrors(t *testing.T) { {resolve, ".b", ".b", ""}, {resolve, "\u3002b", ".b", ""}, {resolve, "..b", "..b", ""}, + {resolve, "b.", "b.", ""}, + {resolve, "ƀ.", "xn--lha.", ""}, {resolve, "b..", "b..", ""}, {resolve, "ƀ..", "xn--lha..", ""}, + {resolve, "b...", "b...", ""}, + {resolve, "ƀ...", "xn--lha...", ""}, {resolve, "\xed", "", "P1"}, // Raw punycode diff --git a/internal/export/idna/idna9.0.0.go b/internal/export/idna/idna9.0.0.go index 698471d7..017335c9 100644 --- a/internal/export/idna/idna9.0.0.go +++ b/internal/export/idna/idna9.0.0.go @@ -348,10 +348,10 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { } // It seems like we should only create this error on ToASCII, but the // UTS 46 conformance tests suggests we should always check this. - if err == nil && p.verifyDNSLength && s == "" { + if err == nil && p.verifyDNSLength && (s == "" || s[len(s)-1] == '.') { err = &labelError{s, "A4"} } - labels := labelIter{orig: s, verifyDNSLength: p.verifyDNSLength} + labels := labelIter{orig: s} for ; !labels.done(); labels.next() { label := labels.label() if label == "" { @@ -404,12 +404,9 @@ func (p *Profile) process(s string, toASCII bool) (string, error) { } s = labels.result() if toASCII && p.verifyDNSLength && err == nil { - // Compute the length of the domain name minus the root label and its dot. + // Compute the length of the domain name. n := len(s) - if n > 0 && s[n-1] == '.' { - n-- - } - if len(s) < 1 || n > 253 { + if n < 1 || n > 253 { err = &labelError{s, "A4"} } } @@ -500,12 +497,11 @@ func validateAndMap(p *Profile, s string) (string, error) { // A labelIter allows iterating over domain name labels. type labelIter struct { - orig string - slice []string - curStart int - curEnd int - i int - verifyDNSLength bool + orig string + slice []string + curStart int + curEnd int + i int } func (l *labelIter) reset() { @@ -537,8 +533,7 @@ func (l *labelIter) label() string { return l.orig[l.curStart:l.curEnd] } -// next sets the value to the next label. It skips the last label if it is empty -// and l.verifyDNSLength is false. +// next sets the value to the next label. It skips the last label if it is empty. func (l *labelIter) next() { l.i++ if l.slice != nil { @@ -547,7 +542,7 @@ func (l *labelIter) next() { } } else { l.curStart = l.curEnd + 1 - if !l.verifyDNSLength && l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { + if l.curStart == len(l.orig)-1 && l.orig[l.curStart] == '.' { l.curStart = len(l.orig) } } diff --git a/internal/export/idna/idna9.0.0_test.go b/internal/export/idna/idna9.0.0_test.go index 26f36225..393e7074 100644 --- a/internal/export/idna/idna9.0.0_test.go +++ b/internal/export/idna/idna9.0.0_test.go @@ -71,15 +71,23 @@ func TestLabelErrors(t *testing.T) { {lengthA, ".b", "b", ""}, {lengthA, "\u3002b", "b", ""}, {lengthA, "..b", "b", ""}, + {lengthA, "b.", "b.", "A4"}, + {lengthA, "ƀ.", "xn--lha.", "A4"}, {lengthA, "b..", "b..", "A4"}, {lengthA, "ƀ..", "xn--lha..", "A4"}, + {lengthA, "b...", "b...", "A4"}, + {lengthA, "ƀ...", "xn--lha...", "A4"}, {resolve, "a..b", "a..b", ""}, {resolve, ".b", "b", ""}, {resolve, "\u3002b", "b", ""}, {resolve, "..b", "b", ""}, + {resolve, "b.", "b.", ""}, + {resolve, "ƀ.", "xn--lha.", ""}, {resolve, "b..", "b..", ""}, {resolve, "ƀ..", "xn--lha..", ""}, + {resolve, "b...", "b...", ""}, + {resolve, "ƀ...", "xn--lha...", ""}, {resolve, "\xed", "", "P1"}, // Raw punycode