From fe9e8a449359b598c9136a444e4e8f65d9ae326f Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 1 Aug 2019 13:52:38 -0400 Subject: [PATCH] MM-17426 Properly specify multiple Accept headers for post metadata (#11766) * MM-17326 Properly specify multiple Accept headers for post metadata * Add q= weight to text/html --- app/post_metadata.go | 3 ++- app/post_metadata_test.go | 42 +++++++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/post_metadata.go b/app/post_metadata.go index 6959b44018..d04a418c29 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -379,7 +379,8 @@ func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool // /api/v4/image requires authentication, so bypass the API by hitting the proxy directly body, contentType, err = a.ImageProxy.GetImageDirect(a.ImageProxy.GetUnproxiedImageURL(request.URL.String())) } else { - request.Header.Add("Accept", "image/*, text/html") + request.Header.Add("Accept", "image/*") + request.Header.Add("Accept", "text/html;q=0.8") client := a.HTTPService.MakeClient(false) client.Timeout = time.Duration(*a.Config().ExperimentalSettings.LinkMetadataTimeoutMilliseconds) * time.Millisecond diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index cae456474a..f5d8db8031 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -1372,26 +1372,35 @@ func TestGetLinkMetadata(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { params := r.URL.Query() - if strings.HasPrefix(r.URL.Path, "/image") { - height, _ := strconv.ParseInt(params["height"][0], 10, 0) - width, _ := strconv.ParseInt(params["width"][0], 10, 0) + writeImage := func(height, width int) { - img := image.NewGray(image.Rect(0, 0, int(width), int(height))) + img := image.NewGray(image.Rect(0, 0, height, width)) var encoder png.Encoder encoder.Encode(w, img) - } else if strings.HasPrefix(r.URL.Path, "/opengraph") { + } + + writeHTML := func(title string) { w.Header().Set("Content-Type", "text/html") w.Write([]byte(` - + `)) + } + + if strings.HasPrefix(r.URL.Path, "/image") { + height, _ := strconv.ParseInt(params["height"][0], 10, 0) + width, _ := strconv.ParseInt(params["width"][0], 10, 0) + + writeImage(int(height), int(width)) + } else if strings.HasPrefix(r.URL.Path, "/opengraph") { + writeHTML(params["title"][0]) } else if strings.HasPrefix(r.URL.Path, "/json") { w.Header().Set("Content-Type", "application/json") @@ -1405,6 +1414,14 @@ func TestGetLinkMetadata(t *testing.T) { case <-r.Context().Done(): } w.Write([]byte("")) + } else if strings.HasPrefix(r.URL.Path, "/mixed") { + for _, acceptedType := range r.Header["Accept"] { + if strings.HasPrefix(acceptedType, "image/*") || strings.HasPrefix(acceptedType, "image/png") { + writeImage(10, 10) + } else if strings.HasPrefix(acceptedType, "text/html") { + writeHTML("mixed") + } + } } else { w.WriteHeader(http.StatusInternalServerError) } @@ -1868,6 +1885,19 @@ func TestGetLinkMetadata(t *testing.T) { assert.NotNil(t, err) assert.IsType(t, imageproxy.Error{}, err) }) + + t.Run("should prefer images for mixed content", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/mixed?name=" + t.Name() + timestamp := int64(1547510400000) + + og, img, err := th.App.getLinkMetadata(requestURL, timestamp, true) + assert.Nil(t, og) + assert.NotNil(t, img) + assert.Nil(t, err) + }) } func TestResolveMetadataURL(t *testing.T) {