[keyserver] Make sure we don't call fetchServerThreadInfos with empty threadIDs
ClosedPublic
Actions

Authored by ashoat on Jul 28 2023, 6:51 AM.

Details

Reviewers

atul
tomek
michal
bartek
kamil
inka
ginsu
varun

Commits

rCOMMc05759090c30: [keyserver] Make sure we don't call fetchServerThreadInfos with empty threadIDs

Summary

This diff fixes a regression introduced in D8509. We should never specify an empty threadIDs parameter to the FetchThreadInfosFilter we pass to fetchServerThreadInfos.

This diff addresses an urgent regression – see ENG-4529 for details.

Test Plan

I confirmed that created a message in a thread with an @-tag of somebody who is already in the thread does not cause the error described in the linked task

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Jul 28 2023, 6:51 AM

ashoat published this revision for review.Jul 28 2023, 6:51 AM

Better to handle this in fetchServerThreadInfos, so it can't happen from any callsite

tomek requested changes to this revision.Jul 28 2023, 7:01 AM

tomek added inline comments.

keyserver/src/fetchers/thread-fetchers.js
81–83 ↗	(On Diff #29170)	Is it really correct? What if `threadIDs` isn't specified in `filter` but `threadID` is?

This revision now requires changes to proceed.Jul 28 2023, 7:01 AM

ashoat requested review of this revision.Jul 28 2023, 7:04 AM

ashoat added inline comments.

keyserver/src/fetchers/thread-fetchers.js
81–83 ↗	(On Diff #29170)	It's correct. If `threadIDs` is specified and empty, we are asking the function to filter to an empty list of `threadIDs`. If `threadID` is specified, there is no way it is empty... it can have precisely 1 value, there is no way for it to have 0 values if it's specified.