From 111e3f2106646cd29f7f74c0102f2a570c598e2e Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 4 Mar 2024 17:07:43 +0100 Subject: [PATCH] Reuse a Reader instead of copying to a buffer when parsing an atom feed --- internal/reader/atom/atom_03_test.go | 16 ++--- internal/reader/atom/atom_10_test.go | 98 ++++++++++++++-------------- internal/reader/atom/parser.go | 13 ++-- internal/reader/atom/parser_test.go | 4 +- 4 files changed, 64 insertions(+), 67 deletions(-) diff --git a/internal/reader/atom/atom_03_test.go b/internal/reader/atom/atom_03_test.go index d9715274..0d21f9c1 100644 --- a/internal/reader/atom/atom_03_test.go +++ b/internal/reader/atom/atom_03_test.go @@ -27,7 +27,7 @@ func TestParseAtom03(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -87,7 +87,7 @@ func TestParseAtom03WithoutFeedTitle(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -110,7 +110,7 @@ func TestParseAtom03WithoutEntryTitleButWithLink(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -138,7 +138,7 @@ func TestParseAtom03WithoutEntryTitleButWithSummary(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -166,7 +166,7 @@ func TestParseAtom03WithoutEntryTitleButWithXMLContent(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -197,7 +197,7 @@ func TestParseAtom03WithSummaryOnly(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -228,7 +228,7 @@ func TestParseAtom03WithXMLContent(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -259,7 +259,7 @@ func TestParseAtom03WithBase64Content(t *testing.T) { ` - feed, err := Parse("http://diveintomark.org/", bytes.NewBufferString(data)) + feed, err := Parse("http://diveintomark.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } diff --git a/internal/reader/atom/atom_10_test.go b/internal/reader/atom/atom_10_test.go index 2e8e8aa5..9a2fd4f4 100644 --- a/internal/reader/atom/atom_10_test.go +++ b/internal/reader/atom/atom_10_test.go @@ -31,7 +31,7 @@ func TestParseAtomSample(t *testing.T) { ` - feed, err := Parse("http://example.org/feed.xml", bytes.NewBufferString(data)) + feed, err := Parse("http://example.org/feed.xml", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -93,7 +93,7 @@ func TestParseFeedWithoutTitle(t *testing.T) { 2003-12-13T18:30:02Z ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -123,7 +123,7 @@ func TestParseEntryWithoutTitleButWithURL(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -154,7 +154,7 @@ func TestParseEntryWithoutTitleButWithSummary(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -187,7 +187,7 @@ func TestParseEntryWithoutTitleButWithXHTMLContent(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -206,7 +206,7 @@ func TestParseFeedURL(t *testing.T) { 2003-12-13T18:30:02Z ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -238,7 +238,7 @@ func TestParseFeedWithRelativeURL(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -272,7 +272,7 @@ func TestParseEntryWithRelativeURL(t *testing.T) { ` - feed, err := Parse("https://example.net/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.net/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -298,7 +298,7 @@ func TestParseEntryURLWithTextHTMLType(t *testing.T) { ` - feed, err := Parse("https://example.net/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.net/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -324,7 +324,7 @@ func TestParseEntryURLWithNoRelAndNoType(t *testing.T) { ` - feed, err := Parse("https://example.net/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.net/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -350,7 +350,7 @@ func TestParseEntryURLWithAlternateRel(t *testing.T) { ` - feed, err := Parse("https://example.net/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.net/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -378,7 +378,7 @@ func TestParseEntryTitleWithWhitespaces(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -412,7 +412,7 @@ func TestParseEntryWithPlainTextTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -459,7 +459,7 @@ func TestParseEntryWithHTMLTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -497,7 +497,7 @@ func TestParseEntryWithXHTMLTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -524,7 +524,7 @@ func TestParseEntryWithEmptyXHTMLTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -551,7 +551,7 @@ func TestParseEntryWithXHTMLTitleWithoutDiv(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -577,7 +577,7 @@ func TestParseEntryWithNumericCharacterReferenceTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -603,7 +603,7 @@ func TestParseEntryWithDoubleEncodedEntitiesTitle(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -629,7 +629,7 @@ func TestParseEntryWithXHTMLSummary(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -671,7 +671,7 @@ func TestParseEntryWithHTMLSummary(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -723,7 +723,7 @@ func TestParseEntryWithTextSummary(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -776,7 +776,7 @@ func TestParseEntryWithTextContent(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -821,7 +821,7 @@ func TestParseEntryWithHTMLContent(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -852,7 +852,7 @@ func TestParseEntryWithXHTMLContent(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -881,7 +881,7 @@ func TestParseEntryWithAuthorName(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -910,7 +910,7 @@ func TestParseEntryWithoutAuthorName(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -941,7 +941,7 @@ func TestParseEntryWithMultipleAuthors(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -969,7 +969,7 @@ func TestParseEntryWithoutAuthor(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1000,7 +1000,7 @@ func TestParseFeedWithMultipleAuthors(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1025,7 +1025,7 @@ func TestParseFeedWithoutAuthor(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1075,7 +1075,7 @@ func TestParseEntryWithEnclosures(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1135,7 +1135,7 @@ func TestParseEntryWithoutEnclosureURL(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1168,7 +1168,7 @@ func TestParseEntryWithPublished(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1194,7 +1194,7 @@ func TestParseEntryWithPublishedAndUpdated(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1206,7 +1206,7 @@ func TestParseEntryWithPublishedAndUpdated(t *testing.T) { func TestParseInvalidXml(t *testing.T) { data := `garbage` - _, err := Parse("https://example.org/", bytes.NewBufferString(data)) + _, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err == nil { t.Error("Parse should returns an error") } @@ -1221,7 +1221,7 @@ func TestParseTitleWithSingleQuote(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1240,7 +1240,7 @@ func TestParseTitleWithEncodedSingleQuote(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1259,7 +1259,7 @@ func TestParseTitleWithSingleQuoteAndHTMLType(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1278,7 +1278,7 @@ func TestParseWithHTMLEntity(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1297,7 +1297,7 @@ func TestParseWithInvalidCharacterEntity(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1330,7 +1330,7 @@ A website: http://example.org/ ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1396,7 +1396,7 @@ A website: http://example.org/ ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1467,7 +1467,7 @@ func TestParseRepliesLinkRelationWithHTMLType(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1511,7 +1511,7 @@ func TestParseRepliesLinkRelationWithXHTMLType(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1550,7 +1550,7 @@ func TestParseRepliesLinkRelationWithNoType(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1590,7 +1590,7 @@ func TestAbsoluteCommentsURL(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1631,7 +1631,7 @@ func TestParseFeedWithCategories(t *testing.T) { ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } @@ -1661,7 +1661,7 @@ func TestParseFeedWithIconURL(t *testing.T) { http://example.org/icon.png ` - feed, err := Parse("https://example.org/", bytes.NewBufferString(data)) + feed, err := Parse("https://example.org/", bytes.NewReader([]byte(data))) if err != nil { t.Fatal(err) } diff --git a/internal/reader/atom/parser.go b/internal/reader/atom/parser.go index 03aa9158..bdc28239 100644 --- a/internal/reader/atom/parser.go +++ b/internal/reader/atom/parser.go @@ -4,7 +4,6 @@ package atom // import "miniflux.app/v2/internal/reader/atom" import ( - "bytes" "encoding/xml" "fmt" "io" @@ -18,25 +17,23 @@ type atomFeed interface { } // Parse returns a normalized feed struct from a Atom feed. -func Parse(baseURL string, r io.Reader) (*model.Feed, error) { - var buf bytes.Buffer - tee := io.TeeReader(r, &buf) - +func Parse(baseURL string, r io.ReadSeeker) (*model.Feed, error) { var rawFeed atomFeed - if getAtomFeedVersion(tee) == "0.3" { + if getAtomFeedVersion(r) == "0.3" { rawFeed = new(atom03Feed) } else { rawFeed = new(atom10Feed) } + r.Seek(0, io.SeekStart) - if err := xml_decoder.NewXMLDecoder(&buf).Decode(rawFeed); err != nil { + if err := xml_decoder.NewXMLDecoder(r).Decode(rawFeed); err != nil { return nil, fmt.Errorf("atom: unable to parse feed: %w", err) } return rawFeed.Transform(baseURL), nil } -func getAtomFeedVersion(data io.Reader) string { +func getAtomFeedVersion(data io.ReadSeeker) string { decoder := xml_decoder.NewXMLDecoder(data) for { token, _ := decoder.Token() diff --git a/internal/reader/atom/parser_test.go b/internal/reader/atom/parser_test.go index eb8ee586..a6e40dcc 100644 --- a/internal/reader/atom/parser_test.go +++ b/internal/reader/atom/parser_test.go @@ -30,7 +30,7 @@ func TestDetectAtom10(t *testing.T) { ` - version := getAtomFeedVersion(bytes.NewBufferString(data)) + version := getAtomFeedVersion(bytes.NewReader([]byte(data))) if version != "1.0" { t.Errorf(`Invalid Atom version detected: %s`, version) } @@ -54,7 +54,7 @@ func TestDetectAtom03(t *testing.T) { ` - version := getAtomFeedVersion(bytes.NewBufferString(data)) + version := getAtomFeedVersion(bytes.NewReader([]byte(data))) if version != "0.3" { t.Errorf(`Invalid Atom version detected: %s`, version) }