PHPLIB-1728: Encryption flag in driver metadata #1809

paulinevos · 2025-12-16T13:45:12Z

Prepends iue for In-Use Encryption to platform metadata string in MongoDB handshake. This will allow for easier detection of encryption usage in higher level libraries.

This PR also extracts the options array into DriverOptions in order to simplify testing the I/O of options as we do quite a bit of transformation on them.

GromNaN

This PR also extracts the options array into DriverOptions in order to simplify testing the I/O of options as we do quite a bit of transformation on them.

This sounds like a good idea, and this has no impact on users, since these classes are internal and cannot be accessed from public APIs.

But I question whether an undefined and a null option have the same meaning in this context.

src/ValueObject/AutoEncryptionOptions.php

src/ValueObject/DriverOptions.php

GromNaN · 2025-12-16T14:06:35Z

src/Model/DriverOptions.php

+        public readonly Encoder $builderEncoder,
+        public readonly array $autoEncryption,
+        public readonly array $driver,
+        private readonly array $miscOptions,


Should it be more specific.
Not $managerOptions because $autoEncryption is a manager option.

I figured no, as if we get more specific we are tying it more to server implementation. I figured that as much as possible we want to pass options as is. In my mind it's just any options that we aren't transforming in some way.

src/ValueObject/DriverOptions.php

GromNaN · 2025-12-16T14:13:59Z

src/Model/DriverOptions.php

+
+        return (new self(
+            typeMap: $options[self::KEY_TYPE_MAP],
+            builderEncoder: $options[self::KEY_BUILDER_ENCODER] ?? new BuilderEncoder(),


You don't validate that builderEncoder is an instance of BuilderEncoder before using it for a typed parameter. This will result in an unspecific TypeError.

I validate that it's an instance of Encoder in ensureValidArrayOptions. There's a test case for it too

src/Client.php

So we can unit test options, as we're transforming some of the options that are passed stuff

Sending extra metadata in the handshake will allow us to detect when encryption is enabled. `iue` for In-Use Ecnryption was chosen to save bytes (as metadata will be truncated if it exceeds byte limit)

paulinevos · 2025-12-16T16:29:43Z

But I question whether an undefined and a null option have the same meaning in this context.

There were some test cases re: this in ClientTest so I ensured that those passed and that that behavior didn't change. As far as other potential nullables, I'm not too sure...

GromNaN

Tests are failing. Do you need help for the spec tests?

GromNaN · 2025-12-17T11:49:54Z

src/Model/DriverOptions.php

+                'autoEncryption' => $this->autoEncryption,
+                'driver' => $this->driver,
+            ] + $this->miscOptions,
+            fn ($option) => $option !== null,


Suggested change

fn ($option) => $option !== null,

static fn ($option) => $option !== null,

GromNaN · 2025-12-17T11:54:01Z

src/Model/DriverOptions.php

+        return new self(
+            typeMap: $this->typeMap,
+            builderEncoder: $this->builderEncoder,
+            autoEncryption: $this->autoEncryption,
+            driver: $mergedDriver,
+            miscOptions: $this->miscOptions,
+        );
+    }


Creating a second instance of DriverOptions does not seem efficient to me, especially since this method is always called in the factory. A simple assignment is enough.

Suggested change

return new self(

typeMap: $this->typeMap,

builderEncoder: $this->builderEncoder,

autoEncryption: $this->autoEncryption,

driver: $mergedDriver,

miscOptions: $this->miscOptions,

);

}

$this->driver = $mergedDriver;

}

The thing is that then I'd have to change the readonly and visibility (which is fine, I can make it private I guess. Nothing is directly accessing it as of now)

If you don't use CPP for $driver, you can alter its value before setting the property.

alcaeus · 2025-12-17T14:59:22Z

src/Model/DriverOptions.php

+
+    public static function fromArray(array $options): self
+    {
+        $options += ['typeMap' => self::DEFAULT_TYPE_MAP];


Should this be self::KEY_TYPE_MAP?

Suggested change

$options += ['typeMap' => self::DEFAULT_TYPE_MAP];

$options += ['self::KEY_TYPE_MAP' => self::DEFAULT_TYPE_MAP];

paulinevos requested a review from a team as a code owner December 16, 2025 13:45

paulinevos requested a review from GromNaN December 16, 2025 13:45

paulinevos force-pushed the 1728-encryption-usage branch from 781e640 to 130f2e6 Compare December 16, 2025 13:55

GromNaN requested changes Dec 16, 2025

View reviewed changes

paulinevos force-pushed the 1728-encryption-usage branch 2 times, most recently from b5c3b2d to 8b56aa2 Compare December 16, 2025 16:23

paulinevos added 3 commits December 16, 2025 17:27

Extract driver options into value objects

38aa388

So we can unit test options, as we're transforming some of the options that are passed stuff

Use DriverOptions value object in Client

39c9c0b

Prepend encryption flag iue to platform metadata

ddeaac1

Sending extra metadata in the handshake will allow us to detect when encryption is enabled. `iue` for In-Use Ecnryption was chosen to save bytes (as metadata will be truncated if it exceeds byte limit)

paulinevos force-pushed the 1728-encryption-usage branch from 8b56aa2 to ddeaac1 Compare December 16, 2025 16:27

paulinevos requested a review from GromNaN December 16, 2025 16:30

GromNaN reviewed Dec 17, 2025

View reviewed changes

alcaeus reviewed Dec 17, 2025

View reviewed changes

	fn ($option) => $option !== null,
	static fn ($option) => $option !== null,

	$options += ['typeMap' => self::DEFAULT_TYPE_MAP];
	$options += ['self::KEY_TYPE_MAP' => self::DEFAULT_TYPE_MAP];

PHPLIB-1728: Encryption flag in driver metadata #1809

Are you sure you want to change the base?

PHPLIB-1728: Encryption flag in driver metadata #1809

Conversation

paulinevos commented Dec 16, 2025

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulinevos Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulinevos commented Dec 16, 2025

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paulinevos Dec 16, 2025 •

edited

Loading