From ba9a1e13b0fb9d4579077b7b37666e950a5fb6ed Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Fri, 4 Aug 2023 11:15:04 +0900 Subject: [PATCH] move json marshalling and error checking into parsOpenGraphMetadata fn --- server/channels/app/opengraph.go | 21 ++++++++------- server/channels/app/opengraph_test.go | 39 +++++++++++++++++++++++++++ server/channels/app/post_metadata.go | 2 +- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/server/channels/app/opengraph.go b/server/channels/app/opengraph.go index 5912f9efa1..477413e485 100644 --- a/server/channels/app/opengraph.go +++ b/server/channels/app/opengraph.go @@ -35,17 +35,11 @@ func (a *App) GetOpenGraphMetadata(requestURL string) ([]byte, error) { } defer res.Body.Close() - graph := a.parseOpenGraphMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) - - ogJSON, err := graph.ToJSON() + _, ogJSON, err := a.parseOpenGraphMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) if err != nil { return nil, err } - if len(ogJSON) > openGraphMetadataCacheEntrySizeLimit { - return nil, errors.New("opengraph data exceeds cache entry size limit") - } - err = a.Srv().openGraphDataCache.SetWithExpiry(requestURL, ogJSON, 1*time.Hour) if err != nil { return nil, err @@ -54,7 +48,7 @@ func (a *App) GetOpenGraphMetadata(requestURL string) ([]byte, error) { return ogJSON, nil } -func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentType string) *opengraph.OpenGraph { +func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, []byte, error) { og := opengraph.NewOpenGraph() body = forceHTMLEncodingToUTF8(io.LimitReader(body, MaxOpenGraphResponseSize), contentType) @@ -76,7 +70,16 @@ func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentT og.URL = requestURL } - return og + ogJSON, err := og.ToJSON() + if err != nil { + return nil, nil, err + } + + if len(ogJSON) > openGraphMetadataCacheEntrySizeLimit { + return nil, nil, errors.New("opengraph data exceeds cache entry size limit") + } + + return og, ogJSON, nil } func forceHTMLEncodingToUTF8(body io.Reader, contentType string) io.Reader { diff --git a/server/channels/app/opengraph_test.go b/server/channels/app/opengraph_test.go index 3f42f60af1..a2d0ecfec3 100644 --- a/server/channels/app/opengraph_test.go +++ b/server/channels/app/opengraph_test.go @@ -4,6 +4,9 @@ package app import ( + "bytes" + "github.com/mattermost/mattermost/server/public/model" + "html/template" "strings" "testing" @@ -135,3 +138,39 @@ func TestOpenGraphDecodeHTMLEntities(t *testing.T) { assert.Equal(t, og.Title, "Test's are the best.©") assert.Equal(t, og.Description, "Test's are the worst.©") } + +func TestParseOpenGraphMetadata(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + opengraphPage := ` + + + + +` + sizeOfJsonExceptTitle := 169 + type Title struct { + Title string + } + + tmpl, err := template.New("Test").Parse(opengraphPage) + assert.NoError(t, err) + + page := new(bytes.Buffer) + title := Title{Title: model.NewRandomString(openGraphMetadataCacheEntrySizeLimit - sizeOfJsonExceptTitle + 1)} + err = tmpl.Execute(page, title) + assert.NoError(t, err) + + _, _, err = th.App.parseOpenGraphMetadata("https://example.com", page, "") + assert.Error(t, err) + assert.Equal(t, err.Error(), "opengraph data exceeds cache entry size limit") + + page.Reset() + title.Title = title.Title[1:] + err = tmpl.Execute(page, title) + assert.NoError(t, err) + + _, _, err = th.App.parseOpenGraphMetadata("https://example.com", page, "") + assert.NoError(t, err) +} diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index a1aa6a93db..ae6996eb85 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -800,7 +800,7 @@ func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType s image, err := parseImages(io.LimitReader(body, MaxMetadataImageSize)) return nil, image, err } else if strings.HasPrefix(contentType, "text/html") { - og := a.parseOpenGraphMetadata(requestURL, body, contentType) + og, _, _ := a.parseOpenGraphMetadata(requestURL, body, contentType) // The OpenGraph library and Go HTML library don't error for malformed input, so check that at least // one of these required fields exists before returning the OpenGraph data