crypto/x509/pkix: print non-standard parsed Names at the end

This doesn't change how ExtraNames are printed, so as not to cause
unnecessary churn of current outputs. Switched the ExtraNames check to a
nil check as we are checking for just-parsed values.

Fixes #39924
Fixes #39873

Change-Id: Ifa07cfc1a057d73643710a774ef8a154222db187
Reviewed-on: https://go-review.googlesource.com/c/go/+/240543
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/doc/go1.15.html b/doc/go1.15.html
index 991314c3a5fc4a41b03da23ef652af8325e87002..ce6894d2e02f04d443cd58fabab7ef2aa13d2081 100644 (file)
--- a/doc/go1.15.html
+++ b/doc/go1.15.html
@@ -553,11 +553,11 @@ Do not send CLs removing the interior tags from such phrases.
 
 <dl id="crypto/x509/pkix"><dt><a href="/pkg/crypto/x509/pkix/">crypto/x509/pkix</a></dt>
   <dd>
-    <p><!-- CL 229864 -->
+    <p><!-- CL 229864, CL 240543 -->
       <a href="/pkg/crypto/x509/pkix/#Name.String"><code>Name.String</code></a>
       now prints non-standard attributes from
       <a href="/pkg/crypto/x509/pkix/#Name.Names"><code>Names</code></a> if
-      <a href="/pkg/crypto/x509/pkix/#Name.ExtraNames"><code>ExtraNames</code></a> is empty.
+      <a href="/pkg/crypto/x509/pkix/#Name.ExtraNames"><code>ExtraNames</code></a> is nil.
     </p>
   </dd>
 </dl><!-- crypto/x509/pkix -->

diff --git a/src/crypto/x509/pkix/pkix.go b/src/crypto/x509/pkix/pkix.go
index 6253a42654b0c5f647e1a4896485bd1eee3d3d19..62ae06549645f7063d9e34951801645266e42e19 100644 (file)
--- a/src/crypto/x509/pkix/pkix.go
+++ b/src/crypto/x509/pkix/pkix.go
@@ -247,20 +247,26 @@ func (n Name) ToRDNSequence() (ret RDNSequence) {
 // String returns the string form of n, roughly following
 // the RFC 2253 Distinguished Names syntax.
 func (n Name) String() string {
-       if len(n.ExtraNames) == 0 {
+       var rdns RDNSequence
+       // If there are no ExtraNames, surface the parsed value (all entries in
+       // Names) instead.
+       if n.ExtraNames == nil {
                for _, atv := range n.Names {
                        t := atv.Type
                        if len(t) == 4 && t[0] == 2 && t[1] == 5 && t[2] == 4 {
                                switch t[3] {
                                case 3, 5, 6, 7, 8, 9, 10, 11, 17:
-                                       // These attributes are already parsed into named fields.
+                                       // These attributes were already parsed into named fields.
                                        continue
                                }
                        }
-                       n.ExtraNames = append(n.ExtraNames, atv)
+                       // Place non-standard parsed values at the beginning of the sequence
+                       // so they will be at the end of the string. See Issue 39924.
+                       rdns = append(rdns, []AttributeTypeAndValue{atv})
                }
        }
-       return n.ToRDNSequence().String()
+       rdns = append(rdns, n.ToRDNSequence()...)
+       return rdns.String()
 }
 
 // oidInAttributeTypeAndValue reports whether a type with the given OID exists

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 0141021504e137f38c60c2eae0dc31a6fa5cc465..840f535e552a34979acca3e8c172b608b3b5fb94 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -2076,10 +2076,31 @@ func TestPKIXNameString(t *testing.T) {
                t.Fatal(err)
        }
 
+       // Check that parsed non-standard attributes are printed.
+       rdns := pkix.Name{
+               Locality: []string{"Gophertown"},
+               ExtraNames: []pkix.AttributeTypeAndValue{
+                       {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "golang.org"}},
+       }.ToRDNSequence()
+       nn := pkix.Name{}
+       nn.FillFromRDNSequence(&rdns)
+
+       // Check that zero-length non-nil ExtraNames hide Names.
+       extra := []pkix.AttributeTypeAndValue{
+               {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "backing array"}}
+       extraNotNil := pkix.Name{
+               Locality:   []string{"Gophertown"},
+               ExtraNames: extra[:0],
+               Names: []pkix.AttributeTypeAndValue{
+                       {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "golang.org"}},
+       }
+
        tests := []struct {
                dn   pkix.Name
                want string
        }{
+               {nn, "L=Gophertown,1.2.3.4.5=#130a676f6c616e672e6f7267"},
+               {extraNotNil, "L=Gophertown"},
                {pkix.Name{
                        CommonName:         "Steve Kille",
                        Organization:       []string{"Isode Limited"},
@@ -2108,6 +2129,20 @@ func TestPKIXNameString(t *testing.T) {
                        ExtraNames: []pkix.AttributeTypeAndValue{
                                {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "golang.org"}},
                }, "1.2.3.4.5=#130a676f6c616e672e6f7267,L=Gophertown"},
+               // If there are no ExtraNames, the Names are printed instead.
+               {pkix.Name{
+                       Locality: []string{"Gophertown"},
+                       Names: []pkix.AttributeTypeAndValue{
+                               {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "golang.org"}},
+               }, "L=Gophertown,1.2.3.4.5=#130a676f6c616e672e6f7267"},
+               // If there are both, print only the ExtraNames.
+               {pkix.Name{
+                       Locality: []string{"Gophertown"},
+                       ExtraNames: []pkix.AttributeTypeAndValue{
+                               {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 5}), Value: "golang.org"}},
+                       Names: []pkix.AttributeTypeAndValue{
+                               {Type: asn1.ObjectIdentifier([]int{1, 2, 3, 4, 6}), Value: "example.com"}},
+               }, "1.2.3.4.5=#130a676f6c616e672e6f7267,L=Gophertown"},
        }
 
        for i, test := range tests {
@@ -2115,6 +2150,10 @@ func TestPKIXNameString(t *testing.T) {
                        t.Errorf("#%d: String() = \n%s\n, want \n%s", i, got, test.want)
                }
        }
+
+       if extra[0].Value != "backing array" {
+               t.Errorf("the backing array of an empty ExtraNames got modified by String")
+       }
 }
 
 func TestRDNSequenceString(t *testing.T) {