Fix system name processing in storage service

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
This commit is contained in:
automated-signal
2026-03-12 15:20:59 -05:00
committed by GitHub
parent aed991f2d4
commit 7830467bbb
5 changed files with 240 additions and 25 deletions

View File

@@ -330,19 +330,19 @@ export async function toContactRecord(
identityState: verified ? toRecordVerified(Number(verified)) : null,
pniSignatureVerified: conversation.get('pniSignatureVerified') ?? false,
profileKey: profileKey ? Bytes.fromBase64(String(profileKey)) : null,
givenName: conversation.get('profileName') ?? null,
familyName: conversation.get('profileFamilyName') ?? null,
givenName: conversation.get('profileName') || null,
familyName: conversation.get('profileFamilyName') || null,
nickname:
nicknameGivenName || nicknameFamilyName
? {
given: nicknameGivenName ?? null,
family: nicknameFamilyName ?? null,
given: nicknameGivenName || null,
family: nicknameFamilyName || null,
}
: null,
note: conversation.get('note') ?? null,
systemGivenName: conversation.get('systemGivenName') ?? null,
systemFamilyName: conversation.get('systemFamilyName') ?? null,
systemNickname: conversation.get('systemNickname') ?? null,
note: conversation.get('note') || null,
systemGivenName: conversation.get('systemGivenName') || null,
systemFamilyName: conversation.get('systemFamilyName') || null,
systemNickname: conversation.get('systemNickname') || null,
blocked: conversation.isBlocked(),
hidden: conversation.get('removalStage') !== undefined,
whitelisted: Boolean(conversation.get('profileSharing')),
@@ -529,10 +529,10 @@ export function toAccountRecord(
return {
profileKey: profileKey ? Bytes.fromBase64(String(profileKey)) : null,
givenName: conversation.get('profileName') ?? null,
familyName: conversation.get('profileFamilyName') ?? null,
avatarUrlPath: itemStorage.get('avatarUrl') ?? null,
username: conversation.get('username') ?? null,
givenName: conversation.get('profileName') || null,
familyName: conversation.get('profileFamilyName') || null,
avatarUrlPath: itemStorage.get('avatarUrl') || null,
username: conversation.get('username') || null,
noteToSelfArchived: Boolean(conversation.get('isArchived')),
noteToSelfMarkedUnread: Boolean(conversation.get('markedUnread')),
readReceipts: getReadReceiptSetting(),
@@ -1323,7 +1323,7 @@ export async function mergeContactRecord(
) ?? null,
} satisfies Proto.ContactRecord.Params;
const e164 = dropNull(contactRecord.e164);
const e164 = dropNull(contactRecord.e164 || null);
const { aci } = contactRecord;
const pni = dropNull(contactRecord.pni);
const pniSignatureVerified = contactRecord.pniSignatureVerified || false;
@@ -1391,8 +1391,8 @@ export async function mergeContactRecord(
);
}
const remoteName = dropNull(contactRecord.givenName);
const remoteFamilyName = dropNull(contactRecord.familyName);
const remoteName = dropNull(contactRecord.givenName || null);
const remoteFamilyName = dropNull(contactRecord.familyName || null);
const localName = conversation.get('profileName');
const localFamilyName = conversation.get('profileFamilyName');
if (
@@ -1416,11 +1416,11 @@ export async function mergeContactRecord(
}
}
conversation.set({
systemGivenName: dropNull(contactRecord.systemGivenName),
systemFamilyName: dropNull(contactRecord.systemFamilyName),
systemNickname: dropNull(contactRecord.systemNickname),
nicknameGivenName: dropNull(contactRecord.nickname?.given),
nicknameFamilyName: dropNull(contactRecord.nickname?.family),
systemGivenName: dropNull(contactRecord.systemGivenName || null),
systemFamilyName: dropNull(contactRecord.systemFamilyName || null),
systemNickname: dropNull(contactRecord.systemNickname || null),
nicknameGivenName: dropNull(contactRecord.nickname?.given || null),
nicknameFamilyName: dropNull(contactRecord.nickname?.family || null),
note: dropNull(contactRecord.note),
});
@@ -1784,7 +1784,7 @@ export async function mergeAccountRecord(
if (Bytes.isNotEmpty(donorSubscriberId)) {
await itemStorage.put('subscriberId', donorSubscriberId);
}
if (typeof donorSubscriberCurrencyCode === 'string') {
if (donorSubscriberCurrencyCode) {
await itemStorage.put(
'subscriberCurrencyCode',
donorSubscriberCurrencyCode
@@ -1968,7 +1968,7 @@ export async function mergeAccountRecord(
const oldStorageID = conversation.get('storageID');
const oldStorageVersion = conversation.get('storageVersion');
if (username !== conversation.get('username')) {
if ((username || undefined) !== conversation.get('username')) {
// Username is part of key transparency self monitor parameters. Make sure
// we delay self-check until the changes fully propagate to the log.
drop(keyTransparency.onKnownIdentifierChange());
@@ -1998,7 +1998,7 @@ export async function mergeAccountRecord(
{ viaStorageServiceSync: true, reason: 'mergeAccountRecord' }
);
const avatarUrl = dropNull(accountRecord.avatarUrlPath);
const avatarUrl = dropNull(accountRecord.avatarUrlPath || null);
await conversation.setAndMaybeFetchProfileAvatar({
avatarUrl,
decryptionKey: profileKey,

View File

@@ -0,0 +1,96 @@
// Copyright 2026 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import z from 'zod';
import type { WritableDB } from '../Interface.std.js';
import type { LoggerType } from '../../types/Logging.std.js';
import { safeParseUnknown } from '../../util/schemas.std.js';
const toNullable = z
.string()
.optional()
.transform(value => value || undefined);
const ConversationSchema = z
.object({
e164: toNullable,
name: toNullable,
profileName: toNullable,
profileFamilyName: toNullable,
systemGivenName: toNullable,
systemFamilyName: toNullable,
systemNickname: toNullable,
nicknameGivenName: toNullable,
nicknameFamilyName: toNullable,
username: toNullable,
})
.passthrough();
export default function updateToSchemaVersion1680(
db: WritableDB,
logger: LoggerType
): void {
const rows: Array<{
id: string;
json: string;
e164?: string;
name?: string;
profileName?: string;
profileFamilyName?: string;
}> = db.prepare('SELECT * FROM conversations').all();
const update = db.prepare(`
UPDATE conversations
SET
json = $json,
e164 = $e164,
name = $name,
profileName = $profileName,
profileFamilyName = $profileFamilyName
WHERE
id is $id
`);
let changes = 0;
for (const row of rows) {
const parse = safeParseUnknown(
ConversationSchema,
JSON.parse(row.json) as unknown
);
if (!parse.success) {
logger.warn(`failed to parse conversation json ${row.id}`, parse.error);
continue;
}
const json = JSON.stringify(parse.data);
const e164 = row.e164 || null;
const name = row.name || null;
const profileName = row.profileName || null;
const profileFamilyName = row.profileFamilyName || null;
if (
json === row.json &&
e164 === row.e164 &&
name === row.name &&
profileName === row.profileName &&
profileFamilyName === row.profileFamilyName
) {
continue;
}
changes += 1;
update.run({
id: row.id,
json,
e164,
name,
profileName,
profileFamilyName,
});
}
if (changes !== 0) {
logger.warn(`fixed ${changes} conversations`);
}
}

View File

@@ -144,6 +144,7 @@ import updateToSchemaVersion1640 from './1640-key-transparency.std.js';
import updateToSchemaVersion1650 from './1650-protected-attachments.std.js';
import updateToSchemaVersion1660 from './1660-protected-attachments-non-unique.std.js';
import updateToSchemaVersion1670 from './1670-drop-call-link-epoch.std.js';
import updateToSchemaVersion1680 from './1680-cleanup-empty-strings.std.js';
import { DataWriter } from '../Server.node.js';
@@ -1648,6 +1649,7 @@ export const SCHEMA_VERSIONS: ReadonlyArray<SchemaUpdateType> = [
{ version: 1650, update: updateToSchemaVersion1650 },
{ version: 1660, update: updateToSchemaVersion1660 },
{ version: 1670, update: updateToSchemaVersion1670 },
{ version: 1680, update: updateToSchemaVersion1680 },
];
export class DBVersionFromFutureError extends Error {

View File

@@ -0,0 +1,117 @@
// Copyright 2026 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import type { WritableDB } from '../../sql/Interface.std.js';
import {
createDB,
getTableData,
insertData,
updateToVersion,
} from './helpers.node.js';
describe('SQL/updateToSchemaVersion1680', () => {
let db: WritableDB;
beforeEach(() => {
db = createDB();
updateToVersion(db, 1670);
});
afterEach(() => {
db.close();
});
it('nulls empty string columns', () => {
insertData(db, 'conversations', [
{
id: 'c1',
e164: '',
name: '',
profileName: '',
profileFamilyName: '',
expireTimerVersion: 1,
json: {},
},
]);
updateToVersion(db, 1680);
assert.deepStrictEqual(getTableData(db, 'conversations'), [
{
id: 'c1',
expireTimerVersion: 1,
json: {},
},
]);
});
it('leaves non-empty columns untouched', () => {
insertData(db, 'conversations', [
{
id: 'c1',
e164: 'abc',
name: 'def',
profileName: 'ghi',
profileFamilyName: 'jkl',
expireTimerVersion: 1,
json: {},
},
]);
updateToVersion(db, 1680);
assert.deepStrictEqual(getTableData(db, 'conversations'), [
{
id: 'c1',
e164: 'abc',
name: 'def',
profileName: 'ghi',
profileFamilyName: 'jkl',
expireTimerVersion: 1,
json: {},
},
]);
});
it('nulls empty json keys', () => {
insertData(db, 'conversations', [
{
id: 'c1',
expireTimerVersion: 1,
json: {
e164: '',
name: '',
profileName: '',
profileFamilyName: '',
systemGivenName: '',
systemFamilyName: '',
systemNickname: '',
nicknameGivenName: '',
nicknameFamilyName: '',
username: '',
// Not on the list of the affected keys
accessKey: '',
// Not a string
hideStory: true,
},
},
]);
updateToVersion(db, 1680);
assert.deepStrictEqual(getTableData(db, 'conversations'), [
{
id: 'c1',
expireTimerVersion: 1,
json: {
accessKey: '',
hideStory: true,
},
},
]);
});
});

View File

@@ -15,5 +15,5 @@ export const isInSystemContacts = ({
// `direct` for redux, `private` for models and the database
(type === 'direct' || type === 'private') &&
(typeof name === 'string' ||
typeof systemGivenName === 'string' ||
typeof systemFamilyName === 'string');
(typeof systemGivenName === 'string' && systemGivenName.length > 0) ||
(typeof systemFamilyName === 'string' && systemFamilyName.length > 0));