Handle errors related to mis-parsing times and dates.

These arent worth crashing the server over, but the implementation of error handling means that's what'll happen today: any error either results in a specific http error or a 502 which effectively blocks progress.
PlayTechnique · Jun 30, 2024 · b78e30a · b78e30a
1 parent bec4826
commit b78e30a
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 4 deletions.
diff --git a/andrew_server_test.go b/andrew_server_test.go
@@ -533,6 +533,57 @@ func TestArticlesOrderInAndrewTableOfContentsIsOverridable(t *testing.T) {
 
 }
 
+// TestInvalidMetaContentDoesNotCrashTheWebServer checks that if there's
+// garbage data inside a meta element named andrew-publish-at that we do
+// something sensible rather than crashing the web server and emitting a 502.
+func TestInvalidAndrewPublishTimeContentDoesNotCrashTheWebServer(t *testing.T) {
+	t.Parallel()
+
+	contentRoot := fstest.MapFS{
+		"index.html": &fstest.MapFile{Data: []byte(`
+<!doctype HTML>
+<head> </head>
+<body>
+{{ .AndrewTableOfContents }}
+</body>
+`)},
+		"a.html": &fstest.MapFile{Data: []byte(`
+<!doctype HTML>
+<head>
+<meta name="andrew-publish-time" content="<no value>"
+</head>
+`)},
+	}
+
+	s := newTestAndrewServer(t, contentRoot)
+
+	resp, err := http.Get(s.BaseUrl)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if resp.StatusCode != 200 {
+		t.Errorf("Expected http 200, received %d", resp.StatusCode)
+	}
+
+	received, err := io.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedIndex := `
+<!doctype HTML>
+<head> </head>
+<body>
+<a class="andrewtableofcontentslink" id="andrewtableofcontentslink0" href="a.html">a.html</a>
+</body>
+`
+
+	if !slices.Equal(received, []byte(expectedIndex)) {
+		t.Fatalf("Diff of Expected and Actual: %s", cmp.Diff(expectedIndex, received))
+	}
+}
+
 // newTestAndrewServer starts an andrew and returns the localhost url that you can run http gets against
 // to retrieve data from that server
 func newTestAndrewServer(t *testing.T, contentRoot fs.FS) *andrew.Server {

diff --git a/page.go b/page.go
@@ -67,10 +67,10 @@ func NewPage(server Server, pageUrl string) (Page, error) {
 	if ok {
 		andrewCreatedAt, err := time.Parse(time.DateOnly, publishTime)
 
-		if err != nil {
-			return Page{}, err
-			// log.Logger("could not parse meta tag andrew-publish-time using time.Parse. Defaulting to mod time")
-		} else {
+		// The errors that come out of time.Parse are all not interesting to me; we just want
+		// to use those errors to tell us if it's safe to set PublishTime to the value of the
+		// meta element.
+		if err == nil {
 			page.PublishTime = andrewCreatedAt
 		}
 	}