Skip to content

Commit

Permalink
MM-53764 - Fix: Improve limits on Opengraph Data Cache (#24177)
Browse files Browse the repository at this point in the history
* 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 095bcd4.

* Revert "fix linting"

This reverts commit f3e1f7b.

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

This reverts commit ba9a1e1.

* Revert "enforce strict opengraph cache entry size limit"

This reverts commit d1de4a8.

* remove /opengraph api endpoint

* i18n

* removing unneeded action and reducer
  • Loading branch information
cpoile committed Aug 17, 2023
1 parent ad142c9 commit 7b0b0d8
Show file tree
Hide file tree
Showing 12 changed files with 0 additions and 253 deletions.
1 change: 0 additions & 1 deletion api/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 0 additions & 38 deletions api/v4/source/opengraph.yaml

This file was deleted.

5 changes: 0 additions & 5 deletions server/channels/api4/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()
Expand Down
41 changes: 0 additions & 41 deletions server/channels/api4/openGraph.go

This file was deleted.

77 changes: 0 additions & 77 deletions server/channels/api4/openGraph_test.go

This file was deleted.

4 changes: 0 additions & 4 deletions server/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
19 changes: 0 additions & 19 deletions server/public/model/client4.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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').
Expand Down
23 changes: 0 additions & 23 deletions webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions webapp/platform/client/src/client4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down

0 comments on commit 7b0b0d8

Please sign in to comment.