From 5ade78ddb311663df90a6e7a81ea4520a1408da9 Mon Sep 17 00:00:00 2001 From: dan Date: Tue, 10 Dec 2024 19:45:04 +0000 Subject: [PATCH 1/3] Add fast path skipping UTF8 length counting (#2819) * Harden UTF8 length test cases * Harden tests to account for new fast path * Add fast paths that skip UTF8 encoding --- packages/lexicon/src/validators/primitives.ts | 59 ++++-- packages/lexicon/tests/_scaffolds/lexicons.ts | 18 ++ packages/lexicon/tests/general.test.ts | 191 +++++++++++++++++- 3 files changed, 246 insertions(+), 22 deletions(-) diff --git a/packages/lexicon/src/validators/primitives.ts b/packages/lexicon/src/validators/primitives.ts index 007dc7be234..81728279e02 100644 --- a/packages/lexicon/src/validators/primitives.ts +++ b/packages/lexicon/src/validators/primitives.ts @@ -198,27 +198,52 @@ export function string( } // maxLength and minLength - if (typeof def.maxLength === 'number' || typeof def.minLength === 'number') { - const len = utf8Len(value) + if (typeof def.minLength === 'number' || typeof def.maxLength === 'number') { + // If the JavaScript string length * 3 is below the maximum limit, + // its UTF8 length (which <= .length * 3) will also be below. + if (typeof def.minLength === 'number' && value.length * 3 < def.minLength) { + return { + success: false, + error: new ValidationError( + `${path} must not be shorter than ${def.minLength} characters`, + ), + } + } - if (typeof def.maxLength === 'number') { - if (len > def.maxLength) { - return { - success: false, - error: new ValidationError( - `${path} must not be longer than ${def.maxLength} characters`, - ), + // If the JavaScript string length * 3 is within the maximum limit, + // its UTF8 length (which <= .length * 3) will also be within. + // When there's no minimal length, this lets us skip the UTF8 length check. + let canSkipUtf8LenChecks = false + if ( + typeof def.minLength === 'undefined' && + typeof def.maxLength === 'number' && + value.length * 3 <= def.maxLength + ) { + canSkipUtf8LenChecks = true + } + + if (!canSkipUtf8LenChecks) { + const len = utf8Len(value) + + if (typeof def.maxLength === 'number') { + if (len > def.maxLength) { + return { + success: false, + error: new ValidationError( + `${path} must not be longer than ${def.maxLength} characters`, + ), + } } } - } - if (typeof def.minLength === 'number') { - if (len < def.minLength) { - return { - success: false, - error: new ValidationError( - `${path} must not be shorter than ${def.minLength} characters`, - ), + if (typeof def.minLength === 'number') { + if (len < def.minLength) { + return { + success: false, + error: new ValidationError( + `${path} must not be shorter than ${def.minLength} characters`, + ), + } } } } diff --git a/packages/lexicon/tests/_scaffolds/lexicons.ts b/packages/lexicon/tests/_scaffolds/lexicons.ts index d0cf414ccef..cc9cede6ef9 100644 --- a/packages/lexicon/tests/_scaffolds/lexicons.ts +++ b/packages/lexicon/tests/_scaffolds/lexicons.ts @@ -313,6 +313,24 @@ const lexicons: LexiconDoc[] = [ }, }, }, + { + lexicon: 1, + id: 'com.example.stringLengthNoMinLength', + defs: { + main: { + type: 'record', + record: { + type: 'object', + properties: { + string: { + type: 'string', + maxLength: 4, + }, + }, + }, + }, + }, + }, { lexicon: 1, id: 'com.example.stringLengthGrapheme', diff --git a/packages/lexicon/tests/general.test.ts b/packages/lexicon/tests/general.test.ts index 2d493a23d09..abd337a7329 100644 --- a/packages/lexicon/tests/general.test.ts +++ b/packages/lexicon/tests/general.test.ts @@ -567,26 +567,207 @@ describe('Record validation', () => { }) it('Applies string length constraint', () => { + // Shorter than two UTF8 characters + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '', + }), + ).toThrow('Record/string must not be shorter than 2 characters') + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'a', + }), + ).toThrow('Record/string must not be shorter than 2 characters') + + // Two to four UTF8 characters + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'ab', + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '\u0301', // Combining acute accent (2 bytes) + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'a\u0301', // 'a' + combining acute accent (1 + 2 bytes = 3 bytes) + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'aé', // 'a' (1 byte) + 'é' (2 bytes) = 3 bytes + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'abc', + }) lex.assertValidRecord('com.example.stringLength', { $type: 'com.example.stringLength', - string: '123', + string: '一', // CJK character (3 bytes) }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '\uD83D', // Unpaired high surrogate (3 bytes) + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'abcd', + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'éé', // 'é' + 'é' (2 + 2 bytes = 4 bytes) + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'aaé', // 1 + 1 + 2 = 4 bytes + }) + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '👋', // 4 bytes + }) + expect(() => lex.assertValidRecord('com.example.stringLength', { $type: 'com.example.stringLength', - string: '1', + string: 'abcde', }), - ).toThrow('Record/string must not be shorter than 2 characters') + ).toThrow('Record/string must not be longer than 4 characters') expect(() => lex.assertValidRecord('com.example.stringLength', { $type: 'com.example.stringLength', - string: '12345', + string: 'a\u0301\u0301', // 1 + (2 * 2) = 5 bytes }), ).toThrow('Record/string must not be longer than 4 characters') expect(() => lex.assertValidRecord('com.example.stringLength', { $type: 'com.example.stringLength', - string: '👨‍👩‍👧‍👧', + string: '\uD83D\uD83D', // Two unpaired high surrogates (3 * 2 = 6 bytes) + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: 'ééé', // 2 + 2 + 2 bytes = 6 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '👋a', // 4 + 1 bytes = 5 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '👨👨', // 4 + 4 = 8 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLength', { + $type: 'com.example.stringLength', + string: '👨‍👩‍👧‍👧', // 4 emojis × 4 bytes + 3 ZWJs × 3 bytes = 25 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + }) + + it('Applies string length constraint (no minLength)', () => { + // Shorter than two UTF8 characters + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '', + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'a', + }) + + // Two to four UTF8 characters + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'ab', + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '\u0301', // Combining acute accent (2 bytes) + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'a\u0301', // 'a' + combining acute accent (1 + 2 bytes = 3 bytes) + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'aé', // 'a' (1 byte) + 'é' (2 bytes) = 3 bytes + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'abc', + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '一', // CJK character (3 bytes) + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '\uD83D', // Unpaired high surrogate (3 bytes) + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'abcd', + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'éé', // 'é' + 'é' (2 + 2 bytes = 4 bytes) + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'aaé', // 1 + 1 + 2 = 4 bytes + }) + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '👋', // 4 bytes + }) + + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'abcde', + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'a\u0301\u0301', // 1 + (2 * 2) = 5 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '\uD83D\uD83D', // Two unpaired high surrogates (3 * 2 = 6 bytes) + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: 'ééé', // 2 + 2 + 2 bytes = 6 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '👋a', // 4 + 1 bytes = 5 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '👨👨', // 4 + 4 = 8 bytes + }), + ).toThrow('Record/string must not be longer than 4 characters') + expect(() => + lex.assertValidRecord('com.example.stringLengthNoMinLength', { + $type: 'com.example.stringLengthNoMinLength', + string: '👨‍👩‍👧‍👧', // 4 emojis × 4 bytes + 3 ZWJs × 3 bytes = 25 bytes }), ).toThrow('Record/string must not be longer than 4 characters') }) From 9fd65ba0fa4caca59fd0e6156145e4c2618e3a95 Mon Sep 17 00:00:00 2001 From: dan Date: Tue, 10 Dec 2024 20:12:47 +0000 Subject: [PATCH 2/3] Add changeset for #2819 (#3223) --- .changeset/perfect-dodos-prove.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/perfect-dodos-prove.md diff --git a/.changeset/perfect-dodos-prove.md b/.changeset/perfect-dodos-prove.md new file mode 100644 index 00000000000..66f7e06971b --- /dev/null +++ b/.changeset/perfect-dodos-prove.md @@ -0,0 +1,5 @@ +--- +"@atproto/lexicon": patch +--- + +Add fast paths that skip UTF8 encoding From 207728d2b3b819af297ecb90e6373eb7721cbe34 Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 11 Dec 2024 19:46:14 +0000 Subject: [PATCH 3/3] Add reasons param to listNotifications (#3222) * Add filter param to listNotifications * Codegen * Changeset * update schemas * tweak schema * filter -> reasons --------- Co-authored-by: dholms --- .changeset/stupid-feet-allow.md | 8 ++++++++ lexicons/app/bsky/notification/listNotifications.json | 8 ++++++++ packages/api/src/client/lexicons.ts | 9 +++++++++ .../types/app/bsky/notification/listNotifications.ts | 2 ++ packages/bsky/src/lexicon/lexicons.ts | 9 +++++++++ .../types/app/bsky/notification/listNotifications.ts | 2 ++ packages/ozone/src/lexicon/lexicons.ts | 9 +++++++++ .../types/app/bsky/notification/listNotifications.ts | 2 ++ packages/pds/src/lexicon/lexicons.ts | 9 +++++++++ .../types/app/bsky/notification/listNotifications.ts | 2 ++ 10 files changed, 60 insertions(+) create mode 100644 .changeset/stupid-feet-allow.md diff --git a/.changeset/stupid-feet-allow.md b/.changeset/stupid-feet-allow.md new file mode 100644 index 00000000000..1aef729861a --- /dev/null +++ b/.changeset/stupid-feet-allow.md @@ -0,0 +1,8 @@ +--- +"@atproto/ozone": patch +"@atproto/bsky": patch +"@atproto/api": patch +"@atproto/pds": patch +--- + +Add optional reasons param to listNotifications diff --git a/lexicons/app/bsky/notification/listNotifications.json b/lexicons/app/bsky/notification/listNotifications.json index c85a516723d..7ae3cf9c324 100644 --- a/lexicons/app/bsky/notification/listNotifications.json +++ b/lexicons/app/bsky/notification/listNotifications.json @@ -8,6 +8,14 @@ "parameters": { "type": "params", "properties": { + "reasons": { + "description": "Notification reasons to include in response.", + "type": "array", + "items": { + "type": "string", + "description": "A reason that matches the reason property of #notification." + } + }, "limit": { "type": "integer", "minimum": 1, diff --git a/packages/api/src/client/lexicons.ts b/packages/api/src/client/lexicons.ts index 954fc444c0e..1e616f1b284 100644 --- a/packages/api/src/client/lexicons.ts +++ b/packages/api/src/client/lexicons.ts @@ -8944,6 +8944,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + reasons: { + description: 'Notification reasons to include in response.', + type: 'array', + items: { + type: 'string', + description: + 'A reason that matches the reason property of #notification.', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/api/src/client/types/app/bsky/notification/listNotifications.ts b/packages/api/src/client/types/app/bsky/notification/listNotifications.ts index 10a5b7148f2..92b3a27fece 100644 --- a/packages/api/src/client/types/app/bsky/notification/listNotifications.ts +++ b/packages/api/src/client/types/app/bsky/notification/listNotifications.ts @@ -10,6 +10,8 @@ import * as AppBskyActorDefs from '../actor/defs' import * as ComAtprotoLabelDefs from '../../../com/atproto/label/defs' export interface QueryParams { + /** Notification reasons to include in response. */ + reasons?: string[] limit?: number priority?: boolean cursor?: string diff --git a/packages/bsky/src/lexicon/lexicons.ts b/packages/bsky/src/lexicon/lexicons.ts index 4a2fad8d587..140e617917b 100644 --- a/packages/bsky/src/lexicon/lexicons.ts +++ b/packages/bsky/src/lexicon/lexicons.ts @@ -8944,6 +8944,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + reasons: { + description: 'Notification reasons to include in response.', + type: 'array', + items: { + type: 'string', + description: + 'A reason that matches the reason property of #notification.', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/bsky/src/lexicon/types/app/bsky/notification/listNotifications.ts b/packages/bsky/src/lexicon/types/app/bsky/notification/listNotifications.ts index 70adc412701..88fe90a0518 100644 --- a/packages/bsky/src/lexicon/types/app/bsky/notification/listNotifications.ts +++ b/packages/bsky/src/lexicon/types/app/bsky/notification/listNotifications.ts @@ -11,6 +11,8 @@ import * as AppBskyActorDefs from '../actor/defs' import * as ComAtprotoLabelDefs from '../../../com/atproto/label/defs' export interface QueryParams { + /** Notification reasons to include in response. */ + reasons?: string[] limit: number priority?: boolean cursor?: string diff --git a/packages/ozone/src/lexicon/lexicons.ts b/packages/ozone/src/lexicon/lexicons.ts index 954fc444c0e..1e616f1b284 100644 --- a/packages/ozone/src/lexicon/lexicons.ts +++ b/packages/ozone/src/lexicon/lexicons.ts @@ -8944,6 +8944,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + reasons: { + description: 'Notification reasons to include in response.', + type: 'array', + items: { + type: 'string', + description: + 'A reason that matches the reason property of #notification.', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/ozone/src/lexicon/types/app/bsky/notification/listNotifications.ts b/packages/ozone/src/lexicon/types/app/bsky/notification/listNotifications.ts index 70adc412701..88fe90a0518 100644 --- a/packages/ozone/src/lexicon/types/app/bsky/notification/listNotifications.ts +++ b/packages/ozone/src/lexicon/types/app/bsky/notification/listNotifications.ts @@ -11,6 +11,8 @@ import * as AppBskyActorDefs from '../actor/defs' import * as ComAtprotoLabelDefs from '../../../com/atproto/label/defs' export interface QueryParams { + /** Notification reasons to include in response. */ + reasons?: string[] limit: number priority?: boolean cursor?: string diff --git a/packages/pds/src/lexicon/lexicons.ts b/packages/pds/src/lexicon/lexicons.ts index 954fc444c0e..1e616f1b284 100644 --- a/packages/pds/src/lexicon/lexicons.ts +++ b/packages/pds/src/lexicon/lexicons.ts @@ -8944,6 +8944,15 @@ export const schemaDict = { parameters: { type: 'params', properties: { + reasons: { + description: 'Notification reasons to include in response.', + type: 'array', + items: { + type: 'string', + description: + 'A reason that matches the reason property of #notification.', + }, + }, limit: { type: 'integer', minimum: 1, diff --git a/packages/pds/src/lexicon/types/app/bsky/notification/listNotifications.ts b/packages/pds/src/lexicon/types/app/bsky/notification/listNotifications.ts index 70adc412701..88fe90a0518 100644 --- a/packages/pds/src/lexicon/types/app/bsky/notification/listNotifications.ts +++ b/packages/pds/src/lexicon/types/app/bsky/notification/listNotifications.ts @@ -11,6 +11,8 @@ import * as AppBskyActorDefs from '../actor/defs' import * as ComAtprotoLabelDefs from '../../../com/atproto/label/defs' export interface QueryParams { + /** Notification reasons to include in response. */ + reasons?: string[] limit: number priority?: boolean cursor?: string