MM-53764 - Fix: Improve limits on Opengraph Data Cache (#24177)

* enforce strict opengraph cache entry size limit

* move json marshalling and error checking into parsOpenGraphMetadata fn

* fix linting

* fix potential nil deref

* Revert "fix potential nil deref"

This reverts commit 095bcd496e.

* Revert "fix linting"

This reverts commit f3e1f7b276.

* Revert "move json marshalling and error checking into parsOpenGraphMetadata fn"

This reverts commit ba9a1e13b0.

* Revert "enforce strict opengraph cache entry size limit"

This reverts commit d1de4a8fa4.

* remove /opengraph api endpoint

* i18n

* removing unneeded action and reducer
This commit is contained in:
Christopher Poile
2023-08-17 18:23:39 -04:00
committed by GitHub
parent ad142c958e
commit 7b0b0d8609
12 changed files with 0 additions and 253 deletions

View File

@@ -41,7 +41,6 @@ build-v4: node_modules playbooks
@cat $(V4_SRC)/schemes.yaml >> $(V4_YAML)
@cat $(V4_SRC)/service_terms.yaml >> $(V4_YAML)
@cat $(V4_SRC)/sharedchannels.yaml >> $(V4_YAML)
@cat $(V4_SRC)/opengraph.yaml >> $(V4_YAML)
@cat $(V4_SRC)/reactions.yaml >> $(V4_YAML)
@cat $(V4_SRC)/actions.yaml >> $(V4_YAML)
@cat $(V4_SRC)/bots.yaml >> $(V4_YAML)

View File

@@ -1,38 +0,0 @@
/api/v4/opengraph:
post:
tags:
- OpenGraph
summary: Get open graph metadata for url
description: >
Get Open Graph Metadata for a specif URL. Use the Open Graph protocol to
get some generic metadata about a URL. Used for creating link previews.
__Minimum server version__: 3.10
##### Permissions
No permission required but must be logged in.
operationId: OpenGraph
requestBody:
content:
application/json:
schema:
type: object
required:
- url
properties:
url:
type: string
description: The URL to get Open Graph Metadata.
required: true
responses:
"200":
description: Open Graph retrieval successful
content:
application/json:
schema:
$ref: "#/components/schemas/OpenGraph"
"501":
$ref: "#/components/responses/NotImplemented"

View File

@@ -81,8 +81,6 @@ type Routes struct {
OAuthApps *mux.Router // 'api/v4/oauth/apps'
OAuthApp *mux.Router // 'api/v4/oauth/apps/{app_id:[A-Za-z0-9]+}'
OpenGraph *mux.Router // 'api/v4/opengraph'
SAML *mux.Router // 'api/v4/saml'
Compliance *mux.Router // 'api/v4/compliance'
Cluster *mux.Router // 'api/v4/cluster'
@@ -240,8 +238,6 @@ func Init(srv *app.Server) (*API, error) {
api.BaseRoutes.ReactionByNameForPostForUser = api.BaseRoutes.PostForUser.PathPrefix("/reactions/{emoji_name:[A-Za-z0-9\\_\\-\\+]+}").Subrouter()
api.BaseRoutes.OpenGraph = api.BaseRoutes.APIRoot.PathPrefix("/opengraph").Subrouter()
api.BaseRoutes.Roles = api.BaseRoutes.APIRoot.PathPrefix("/roles").Subrouter()
api.BaseRoutes.Schemes = api.BaseRoutes.APIRoot.PathPrefix("/schemes").Subrouter()
@@ -294,7 +290,6 @@ func Init(srv *app.Server) (*API, error) {
api.InitEmoji()
api.InitOAuth()
api.InitReaction()
api.InitOpenGraph()
api.InitPlugin()
api.InitRole()
api.InitScheme()

View File

@@ -1,41 +0,0 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package api4
import (
"net/http"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/mlog"
)
func (api *API) InitOpenGraph() {
api.BaseRoutes.OpenGraph.Handle("", api.APISessionRequired(getOpenGraphMetadata)).Methods("POST")
}
func getOpenGraphMetadata(c *Context, w http.ResponseWriter, r *http.Request) {
if !*c.App.Config().ServiceSettings.EnableLinkPreviews {
c.Err = model.NewAppError("getOpenGraphMetadata", "api.post.link_preview_disabled.app_error", nil, "", http.StatusNotImplemented)
return
}
props := model.StringInterfaceFromJSON(r.Body)
url := ""
ok := false
if url, ok = props["url"].(string); url == "" || !ok {
c.SetInvalidParam("url")
return
}
buf, err := c.App.GetOpenGraphMetadata(url)
if err != nil {
mlog.Warn("GetOpenGraphMetadata request failed",
mlog.String("requestURL", url),
mlog.Err(err))
w.Write([]byte(`{"url": ""}`))
return
}
w.Write(buf)
}

View File

@@ -1,77 +0,0 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package api4
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost/server/public/model"
)
func TestGetOpenGraphMetadata(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
client := th.Client
enableLinkPreviews := *th.App.Config().ServiceSettings.EnableLinkPreviews
allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections
defer func() {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableLinkPreviews = enableLinkPreviews })
th.App.UpdateConfig(func(cfg *model.Config) {
cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections
})
}()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableLinkPreviews = true })
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
})
ogDataCacheMissCount := 0
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ogDataCacheMissCount++
if r.URL.Path == "/og-data/" {
fmt.Fprintln(w, `
<html><head><meta property="og:type" content="article" />
<meta property="og:title" content="Test Title" />
<meta property="og:url" content="http://example.com/" />
</head><body></body></html>
`)
} else if r.URL.Path == "/no-og-data/" {
fmt.Fprintln(w, `<html><head></head><body></body></html>`)
}
}))
for _, data := range [](map[string]any){
{"path": "/og-data/", "title": "Test Title", "cacheMissCount": 1},
{"path": "/no-og-data/", "title": "", "cacheMissCount": 2},
// Data should be cached for following
{"path": "/og-data/", "title": "Test Title", "cacheMissCount": 2},
{"path": "/no-og-data/", "title": "", "cacheMissCount": 2},
} {
openGraph, _, err := client.OpenGraph(context.Background(), ts.URL+data["path"].(string))
require.NoError(t, err)
require.Equalf(t, openGraph["title"], data["title"].(string),
"OG data title mismatch for path \"%s\".")
require.Equal(t, ogDataCacheMissCount, data["cacheMissCount"].(int),
"Cache miss count didn't match.")
}
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableLinkPreviews = false })
_, resp, err := client.OpenGraph(context.Background(), ts.URL+"/og-data/")
require.Error(t, err)
CheckNotImplementedStatus(t, resp)
}

View File

@@ -2391,10 +2391,6 @@
"other": "{{.Count}} images sent: {{.Filenames}}"
}
},
{
"id": "api.post.link_preview_disabled.app_error",
"translation": "Link previews have been disabled by the system administrator."
},
{
"id": "api.post.patch_post.can_not_update_post_in_deleted.error",
"translation": "Can not update a post in a deleted channel."

View File

@@ -467,10 +467,6 @@ func (c *Client4) oAuthAppRoute(appId string) string {
return fmt.Sprintf("/oauth/apps/%v", appId)
}
func (c *Client4) openGraphRoute() string {
return "/opengraph"
}
func (c *Client4) jobsRoute() string {
return "/jobs"
}
@@ -6779,21 +6775,6 @@ func (c *Client4) GetSupportedTimezone(ctx context.Context) ([]string, *Response
return timezones, BuildResponse(r), nil
}
// Open Graph Metadata Section
// OpenGraph return the open graph metadata for a particular url if the site have the metadata.
func (c *Client4) OpenGraph(ctx context.Context, url string) (map[string]string, *Response, error) {
requestBody := make(map[string]string)
requestBody["url"] = url
r, err := c.DoAPIPost(ctx, c.openGraphRoute(), MapToJSON(requestBody))
if err != nil {
return nil, BuildResponse(r), err
}
defer closeBody(r)
return MapFromJSON(r.Body), BuildResponse(r), nil
}
// Jobs Section
// GetJob gets a single job.

View File

@@ -47,7 +47,6 @@ export default keyMirror({
RECEIVED_REACTION: null,
RECEIVED_REACTIONS: null,
REACTION_DELETED: null,
RECEIVED_OPEN_GRAPH_METADATA: null,
ADD_MESSAGE_INTO_HISTORY: null,
RESET_HISTORY_INDEX: null,

View File

@@ -1335,36 +1335,6 @@ describe('Actions.Posts', () => {
expect(state.entities.emojis.nonExistentEmoji.has(missingEmojiName)).toBeTruthy();
});
it('getOpenGraphMetadata', async () => {
const {dispatch, getState} = store;
const url = 'https://mattermost.com';
const docs = 'https://docs.mattermost.com/';
nock(Client4.getBaseRoute()).
post('/opengraph').
reply(200, {type: 'article', url: 'https://mattermost.com/', title: 'Mattermost private cloud messaging', description: 'Open source, private cloud\nSlack-alternative, \nWorkplace messaging for web, PCs and phones.'});
await dispatch(Actions.getOpenGraphMetadata(url));
nock(Client4.getBaseRoute()).
post('/opengraph').
reply(200, {type: '', url: '', title: '', description: ''});
await dispatch(Actions.getOpenGraphMetadata(docs));
nock(Client4.getBaseRoute()).
post('/opengraph').
reply(200, undefined);
await dispatch(Actions.getOpenGraphMetadata(docs));
const state = getState();
const metadata = state.entities.posts.openGraph;
expect(metadata).toBeTruthy();
expect(metadata[url]).toBeTruthy();
if (metadata[docs]) {
throw new Error('unexpected metadata[docs]');
}
});
it('doPostAction', async () => {
nock(Client4.getBaseRoute()).
post('/posts/posth67ja7ntdkek6g13dp3wka/actions/action7ja7ntdkek6g13dp3wka').

View File

@@ -1269,29 +1269,6 @@ export function addPostReminder(userId: string, postId: string, timestamp: numbe
};
}
export function getOpenGraphMetadata(url: string) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let data;
try {
data = await Client4.getOpenGraphMetadata(url);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
dispatch(logError(error));
return {error};
}
if (data && (data.url || data.type || data.title || data.description)) {
dispatch({
type: PostTypes.RECEIVED_OPEN_GRAPH_METADATA,
data,
url,
});
}
return {data};
};
}
export function doPostAction(postId: string, actionId: string, selectedOption = '') {
return doPostActionWithCookie(postId, actionId, '', selectedOption);
}

View File

@@ -1357,13 +1357,6 @@ function storeAcknowledgementsForPost(state: any, post: Post) {
export function openGraph(state: RelationOneToOne<Post, Record<string, OpenGraphMetadata>> = {}, action: GenericAction) {
switch (action.type) {
case PostTypes.RECEIVED_OPEN_GRAPH_METADATA: {
const nextState = {...state};
nextState[action.url] = action.data;
return nextState;
}
case PostTypes.RECEIVED_NEW_POST:
case PostTypes.RECEIVED_POST: {
const post = action.data;

View File

@@ -2214,13 +2214,6 @@ export default class Client4 {
return this.searchFilesWithParams(teamId, {terms, is_or_search: isOrSearch});
};
getOpenGraphMetadata = (url: string) => {
return this.doFetch<OpenGraphMetadata>(
`${this.getBaseRoute()}/opengraph`,
{method: 'post', body: JSON.stringify({url})},
);
};
doPostAction = (postId: string, actionId: string, selectedOption = '') => {
return this.doPostActionWithCookie(postId, actionId, '', selectedOption);
};