Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@mongodb-js/zstd": "^7.0.0",
"gcp-metadata": "^7.0.1",
"kerberos": "^7.0.0",
"mongodb-client-encryption": ">=7.0.0 <7.1.0",
"mongodb-client-encryption": "^7.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ^7.1.0 so the latest libomngocrypt always gets picked up?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! It should, great catch

"snappy": "^7.3.2",
"socks": "^2.8.6"
},
Expand Down
54 changes: 43 additions & 11 deletions src/client-side-encryption/client_encryption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,16 @@ export class ClientEncryption {
expressionMode: boolean,
options: ClientEncryptionEncryptOptions
): Promise<Binary> {
const { algorithm, keyId, keyAltName, contentionFactor, queryType, rangeOptions, textOptions } =
options;
const {
algorithm,
keyId,
keyAltName,
contentionFactor,
queryType,
rangeOptions,
stringOptions,
textOptions
} = options;
const contextOptions: ExplicitEncryptionContextOptions = {
expressionMode,
algorithm
Expand Down Expand Up @@ -780,8 +788,9 @@ export class ClientEncryption {
contextOptions.rangeOptions = serialize(rangeOptions);
}

if (typeof textOptions === 'object') {
contextOptions.textOptions = serialize(textOptions);
const resolvedStringOptions = stringOptions ?? textOptions;
Comment thread
PavelSafronov marked this conversation as resolved.
if (typeof resolvedStringOptions === 'object') {
contextOptions.textOptions = serialize(resolvedStringOptions);
}
Comment on lines +791 to 794

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree here, we already preferred new stringOptions, and accepting both we just help potential clients to migrate. Left a comment above with self-review on this specific line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to accept both (I'm not sure we do), then we should flip the order of stringOptions ?? textOptions and prioritize the new setting over the old setting.

@tadjik1 tadjik1 Jun 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't remove it, that's why textOptions is deprecated. And stringOptions has precedence as new standard way to pass these options.
Technically speaking this is our "internal" field, we don't have to rename it, but I just thought it would be more consistent. Godriver does the same here, for example.

I'm ok to add this suggested condition and disallow passing both options at the same time, is that what you suggest?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. When both settings are specified, it seems appropriate here to coalesce between the old and the new.

I was thinking of throwing, but it looks like in the driver we typically only do that for conflicting options, not for renamed ones. Is there a plan to drop the old property? Should we add a deprecation notice, or invoke emitWarningOnce?


const valueBuffer = serialize({ v: value });
Expand Down Expand Up @@ -815,6 +824,8 @@ export interface ClientEncryptionEncryptOptions {
| 'Indexed'
| 'Unindexed'
| 'Range'
| 'String'
/** @deprecated Use `'String'` instead. */
Comment thread
PavelSafronov marked this conversation as resolved.
Comment thread
addaleax marked this conversation as resolved.
| 'TextPreview';

/**
Expand All @@ -833,26 +844,38 @@ export interface ClientEncryptionEncryptOptions {
/**
* The query type.
*/
queryType?: 'equality' | 'range' | 'prefixPreview' | 'suffixPreview' | 'substringPreview';
queryType?:
| 'equality'
| 'range'
| 'prefix'
| 'suffix'
/** @deprecated Use `'prefix'` instead. */
| 'prefixPreview'
/** @deprecated Use `'suffix'` instead. */
| 'suffixPreview'
/** @experimental Public Technical Preview: `substringPreview` is an experimental feature and may break at any time. */
| 'substringPreview';
Comment thread
PavelSafronov marked this conversation as resolved.

/** The index options for a Queryable Encryption field supporting "range" queries.*/
rangeOptions?: RangeOptions;

/** Options for a Queryable Encryption field supporting string queries. Only valid when `algorithm` is `'String'`. */
stringOptions?: StringQueryOptions;

/**
* Options for a Queryable Encryption field supporting text queries. Only valid when `algorithm` is `TextPreview`.
* Options for a Queryable Encryption field supporting text queries. Only valid when `algorithm` is `'String'`.
Comment thread
PavelSafronov marked this conversation as resolved.
*
* @experimental Public Technical Preview: `textPreview` is an experimental feature and may break at any time.
* @deprecated Use `stringOptions` instead.
*/
textOptions?: TextQueryOptions;
textOptions?: StringQueryOptions;
}

/**
* Options for a Queryable Encryption field supporting text queries.
* Options for a Queryable Encryption field supporting string queries.
*
* @public
* @experimental Public Technical Preview: `textPreview` is an experimental feature and may break at any time.
*/
export interface TextQueryOptions {
export interface StringQueryOptions {
/** Indicates that text indexes for this field are case sensitive */
caseSensitive: boolean;
/** Indicates that text indexes for this field are diacritic sensitive. */
Expand All @@ -872,6 +895,9 @@ export interface TextQueryOptions {
strMinQueryLength: Int32 | number;
};

/**
* @experimental Public Technical Preview: `substring` is an experimental feature and may break at any time.
*/
substring?: {
/** The maximum allowed length to insert. */
strMaxLength: Int32 | number;
Expand All @@ -882,6 +908,12 @@ export interface TextQueryOptions {
};
}

/**
* @public
* @deprecated Use {@link StringQueryOptions} instead.
*/
export type TextQueryOptions = StringQueryOptions;

/**
* @public
* @experimental
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ export type {
GCPEncryptionKeyOptions,
KMIPEncryptionKeyOptions,
RangeOptions,
StringQueryOptions,
TextQueryOptions
} from './client-side-encryption/client_encryption';
export {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* https://github.com/mongodb/specifications/blob/master/source/client-side-encryption/tests/README.md#25-test-lookup */

import * as fs from 'node:fs/promises';
import * as path from 'node:path';

Expand Down Expand Up @@ -37,7 +39,7 @@ const newEncryptedClient = ({ configuration }: { configuration: TestConfiguratio
}
);

describe('$lookup support', defaultMetadata, function () {
describe('25. Test $lookup', defaultMetadata, function () {
before(async function () {
const mochaTest = { metadata: defaultMetadata };

Expand Down Expand Up @@ -83,6 +85,7 @@ describe('$lookup support', defaultMetadata, function () {
* db.qe2 with options: { "encryptedFields": "<schema-qe2.json>"}.
* db.no_schema with no options.
* db.no_schema2 with no options.
* db.non_csfle_schema with options: { "validator": { "$jsonSchema": "<schema-non-csfle.json>"}}.
* ```
*/
const collections = [
Expand Down Expand Up @@ -115,6 +118,11 @@ describe('$lookup support', defaultMetadata, function () {
name: 'no_schema2',
options: {},
document: { no_schema2: 'no_schema2' }
},
{
name: 'non_csfle_schema',
options: { validator: { $jsonSchema: await readFixture('schema-non-csfle.json') } },
document: { non_csfle_schema: 'non_csfle_schema' }
}
];

Expand All @@ -126,6 +134,7 @@ describe('$lookup support', defaultMetadata, function () {
unencryptedClient = this.configuration.newClient({}, { writeConcern: { w: 'majority' } });

/**
* Insert documents with encryptedClient:
* ```
* {"csfle": "csfle"} into db.csfle
* Use the unencrypted client to retrieve it. Assert the csfle field is BSON binary.
Expand All @@ -137,12 +146,13 @@ describe('$lookup support', defaultMetadata, function () {
* Use the unencrypted client to retrieve it. Assert the qe2 field is BSON binary.
* {"no_schema": "no_schema"} into db.no_schema
* {"no_schema2": "no_schema2"} into db.no_schema2
* {"non_csfle_schema": "non_csfle_schema"} into db.non_csfle_schema
* ```
*/
for (const { name, document } of collections) {
const { insertedId } = await encryptedClient.db('db').collection(name).insertOne(document);

if (name.startsWith('no_')) continue;
if (name.startsWith('no_') || name === 'non_csfle_schema') continue;

expect(await unencryptedClient.db('db').collection(name).findOne(insertedId))
.to.have.property(Object.keys(document)[0])
Expand Down Expand Up @@ -325,6 +335,11 @@ describe('$lookup support', defaultMetadata, function () {
{ requires: { ...defaultMetadata.requires, mongodb: '>=8.1.0' } }
);

// Case 8 expects one of two error messages depending on mongocryptd/crypt_shared and libmongocrypt versions:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case 8 accepts two error messages because the server changed the error text in 8.2 via MONGOCRYPT-793. See spec prose test 25 Case 8.
The second message (csfleEncryptionSchemas) was not observable before libmongocrypt 1.17.0.

// - mongocryptd/crypt_shared <8.2 or libmongocrypt <1.17.0: "not supported"
// - mongocryptd/crypt_shared 8.2+ and libmongocrypt 1.17.0+:
// "Cannot specify both encryptionInformation and csfleEncryptionSchemas unless
// csfleEncryptionSchemas only contains non-encryption JSON schema validators"
test(
'Case 8: db.csfle joins db.qe',
'csfle',
Expand All @@ -339,7 +354,7 @@ describe('$lookup support', defaultMetadata, function () {
},
{ $project: { _id: 0 } }
],
/not supported/i,
/not supported|Cannot specify both encryptionInformation and csfleEncryptionSchemas/i,
{ requires: { ...defaultMetadata.requires, mongodb: '>=8.1.0' } }
);

Expand All @@ -360,4 +375,26 @@ describe('$lookup support', defaultMetadata, function () {
/Upgrade/i,
{ requires: { ...defaultMetadata.requires, mongodb: '>=7.0.0 <8.1.0' } }
);

// Case 10 requires server 8.2+, mongocryptd/crypt_shared 8.2+, and libmongocrypt 1.17.0+.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case 10 is new in NODE-7221: verifies that a QE collection can $lookup into a collection with a non-encryption JSON schema on server 8.2+. See spec prose test 25 Case 10.

test(
'Case 10: db.qe joins db.non_csfle_schema',
'qe',
[
{ $match: { qe: 'qe' } },
{
$lookup: {
from: 'non_csfle_schema',
as: 'matched',
pipeline: [
{ $match: { non_csfle_schema: 'non_csfle_schema' } },
{ $project: { _id: 0, __safeContent__: 0 } }
]
}
},
{ $project: { _id: 0, __safeContent__: 0 } }
],
{ qe: 'qe', matched: [{ non_csfle_schema: 'non_csfle_schema' }] },
{ requires: { ...defaultMetadata.requires, mongodb: '>=8.2.0' } }
);
});
Loading
Loading