internal/frontend: replace \r\n with \n in readmes and licences

Replace CRLF line terminators in readme and license files before
rendering Overview and Licenses tabs on the Details page. Keeping \r
characters interferes with blackfriday's parser, resulting in an
incorrectly rendered HTML output (see golang/go#40203 for details).

Fixes golang/go#40203.

Change-Id: Id6c2951c9f23e7054957071cf1c33fd3fa6494c6
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/243457
Reviewed-by: Julie Qiu <[email protected]>

Author: akolar

internal/frontend/license.go

@@ -5,6 +5,7 @@
 package frontend
 
 import (
+	"bytes"
 	"context"
 	"sort"
 	"strconv"
@@ -63,6 +64,7 @@ func transformLicenses(modulePath, requestedVersion string, dbLicenses []*licens
 	}
 	anchors := licenseAnchors(filePaths)
 	for i, l := range dbLicenses {
+		l.Contents = bytes.ReplaceAll(l.Contents, []byte("\r"), nil)
 		licenses[i] = License{
 			Anchor:  anchors[i],
 			License: l,

internal/frontend/license_test.go

@@ -5,17 +5,19 @@
 package frontend
 
 import (
+	"bytes"
 	"context"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/safehtml"
 	"golang.org/x/pkgsite/internal/licenses"
 	"golang.org/x/pkgsite/internal/postgres"
 	"golang.org/x/pkgsite/internal/stdlib"
 	"golang.org/x/pkgsite/internal/testing/sample"
+	"golang.org/x/pkgsite/internal/testing/testhelper"
 )
 
 func TestLicenseAnchors(t *testing.T) {
@@ -43,12 +45,27 @@ func TestLicenseAnchors(t *testing.T) {
 func TestFetchLicensesDetails(t *testing.T) {
 	testModule := sample.Module(sample.ModulePath, "v1.2.3", "A/B")
 	stdlibModule := sample.Module(stdlib.ModulePath, "v1.13.0", "cmd/go")
+	crlfPath := "github.com/crlf/module_name"
+	crlfModule := sample.Module(crlfPath, "v1.2.3", "A")
+
 	mit := &licenses.Metadata{Types: []string{"MIT"}, FilePath: "LICENSE"}
 	bsd := &licenses.Metadata{Types: []string{"BSD-3-Clause"}, FilePath: "A/B/LICENSE"}
 
-	mitLicense := &licenses.License{Metadata: mit}
-	bsdLicense := &licenses.License{Metadata: bsd}
+	mitLicense := &licenses.License{
+		Metadata: mit,
+		Contents: []byte(testhelper.MITLicense),
+	}
+	mitLicenseCRLF := &licenses.License{
+		Metadata: mit,
+		Contents: []byte(strings.ReplaceAll(testhelper.MITLicense, "\n", "\r\n")),
+	}
+	bsdLicense := &licenses.License{
+		Metadata: bsd,
+		Contents: []byte(testhelper.BSD0License),
+	}
+
 	testModule.Licenses = []*licenses.License{bsdLicense, mitLicense}
+	crlfModule.Licenses = []*licenses.License{mitLicenseCRLF}
 	sort.Slice(testModule.Directories, func(i, j int) bool {
 		return testModule.Directories[i].Path < testModule.Directories[j].Path
 	})
@@ -68,6 +85,9 @@ func TestFetchLicensesDetails(t *testing.T) {
 	if err := testDB.InsertModule(ctx, stdlibModule); err != nil {
 		t.Fatal(err)
 	}
+	if err := testDB.InsertModule(ctx, crlfModule); err != nil {
+		t.Fatal(err)
+	}
 	for _, test := range []struct {
 		err                                 error
 		name, fullPath, modulePath, version string
@@ -115,6 +135,13 @@ func TestFetchLicensesDetails(t *testing.T) {
 			version:    stdlibModule.Version,
 			want:       stdlibModule.Licenses,
 		},
+		{
+			name:       "module with CRLF line terminators",
+			fullPath:   crlfPath,
+			modulePath: crlfPath,
+			version:    crlfModule.Version,
+			want:       crlfModule.Licenses,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			wantDetails := &LicensesDetails{Licenses: transformLicenses(
@@ -124,11 +151,15 @@ func TestFetchLicensesDetails(t *testing.T) {
 				t.Fatal(err)
 			}
 			if diff := cmp.Diff(wantDetails, got,
-				cmpopts.IgnoreFields(licenses.License{}, "Contents"),
 				cmp.AllowUnexported(safehtml.HTML{}, safehtml.Identifier{}),
 			); diff != "" {
 				t.Errorf("mismatch (-want +got):\n%s", diff)
 			}
+			for _, l := range got.Licenses {
+				if bytes.Contains(l.Contents, []byte("\r")) {
+					t.Errorf("license %s contains \\r line terminators", l.Metadata.FilePath)
+				}
+			}
 		})
 	}
 }

internal/frontend/overview.go

@@ -145,7 +145,8 @@ func ReadmeHTML(ctx context.Context, mi *internal.ModuleInfo, readme *internal.R
 	// Render HTML similar to blackfriday.Run(), but here we implement a custom
 	// Walk function in order to modify image paths in the rendered HTML.
 	b := &bytes.Buffer{}
-	rootNode := parser.Parse([]byte(readme.Contents))
+	contents := bytes.ReplaceAll([]byte(readme.Contents), []byte("\r"), nil)
+	rootNode := parser.Parse(contents)
 	var walkErr error
 	rootNode.Walk(func(node *blackfriday.Node, entering bool) blackfriday.WalkStatus {
 		switch node.Type {

internal/frontend/overview_test.go

@@ -192,6 +192,21 @@ func TestReadmeHTML(t *testing.T) {
 				"<p>It’s part of a demonstration of\n" +
 				`<a href="https://research.swtch.com/vgo1" rel="nofollow">package versioning in Go</a>.</p>`,
 		},
+		{
+			name: "valid markdown readme with CRLF",
+			mi:   &internal.ModuleInfo{},
+			readme: &internal.Readme{
+				Filepath: "README.md",
+				Contents: "This package collects pithy sayings.\r\n\r\n" +
+					"- It's part of a demonstration of\r\n" +
+					"- [package versioning in Go](https://research.swtch.com/vgo1).",
+			},
+			want: "<p>This package collects pithy sayings.</p>\n\n" +
+				"<ul>\n" +
+				"<li>It’s part of a demonstration of</li>\n" +
+				`<li><a href="https://research.swtch.com/vgo1" rel="nofollow">package versioning in Go</a>.</li>` + "\n" +
+				"</ul>\n",
+		},
 		{
 			name: "not markdown readme",
 			mi:   &internal.ModuleInfo{},