Skip to content

Conversation

@paulinevos
Copy link
Contributor

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.

@paulinevos paulinevos requested a review from a team as a code owner December 16, 2025 13:45
@paulinevos paulinevos requested a review from GromNaN December 16, 2025 13:45
@paulinevos paulinevos force-pushed the 1728-encryption-usage branch from 781e640 to 130f2e6 Compare December 16, 2025 13:55
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

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.

@paulinevos paulinevos force-pushed the 1728-encryption-usage branch 3 times, most recently from 8b56aa2 to ddeaac1 Compare December 16, 2025 16:27
@paulinevos
Copy link
Contributor Author

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...

@paulinevos paulinevos requested a review from GromNaN December 16, 2025 16:30
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

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

@paulinevos
Copy link
Contributor Author

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

@GromNaN I might do actually. I can't run tests locally for 8.1 cause pie is throwing an error when I install the extension so I gotta dig into that I think.

@paulinevos paulinevos force-pushed the 1728-encryption-usage branch 5 times, most recently from 3d10d4e to d519d5e Compare December 19, 2025 10:36
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 95.45455% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.88%. Comparing base (82719c4) to head (0213c1e).
⚠️ Report is 1 commits behind head on v2.x.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Client.php 84.21% 3 Missing ⚠️
src/Model/DriverOptions.php 97.10% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               v2.x    #1809      +/-   ##
============================================
+ Coverage     87.74%   87.88%   +0.13%     
- Complexity     3185     3194       +9     
============================================
  Files           424      426       +2     
  Lines          6289     6345      +56     
============================================
+ Hits           5518     5576      +58     
+ Misses          771      769       -2     
Flag Coverage Δ
6.0-replica_set 85.87% <95.45%> (+0.15%) ⬆️
6.0-server 81.90% <93.63%> (+0.19%) ⬆️
6.0-sharded_cluster 85.65% <94.54%> (+0.14%) ⬆️
8.0-replica_set 87.80% <95.45%> (+0.14%) ⬆️
8.0-server 82.74% <93.63%> (+0.18%) ⬆️
8.0-sharded_cluster 87.62% <94.54%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@paulinevos paulinevos force-pushed the 1728-encryption-usage branch 2 times, most recently from d33fe69 to a443ec4 Compare December 19, 2025 11:32
Copilot AI review requested due to automatic review settings December 19, 2025 14:16
@paulinevos paulinevos force-pushed the 1728-encryption-usage branch from a443ec4 to 636e924 Compare December 19, 2025 14:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for indicating In-Use Encryption (IUE) in the MongoDB driver's handshake metadata by prepending "iue" to the platform field when auto-encryption is enabled. The implementation extracts driver options into a dedicated DriverOptions class and auto-encryption options into an AutoEncryptionOptions class to improve testability and maintainability.

Key changes:

  • Created DriverOptions class to encapsulate driver option handling and metadata generation
  • Created AutoEncryptionOptions class to handle encryption-specific options
  • Modified Client class to use the new DriverOptions abstraction

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Model/DriverOptions.php New class that handles driver options, including auto-encryption detection and platform metadata generation with "iue" flag
src/Model/AutoEncryptionOptions.php New class that validates and processes auto-encryption configuration options
src/Client.php Refactored to use DriverOptions and AutoEncryptionOptions classes, removing duplicated option handling logic
tests/Model/DriverOptionsTest.php Comprehensive test coverage for DriverOptions class including encryption flag behavior
tests/Model/AutoEncryptionOptionsTest.php Test coverage for AutoEncryptionOptions class validation and conversion
psalm.xml.dist Added configuration to suppress RedundantConditionGivenDocblockType errors for runtime type checking
psalm-baseline.xml Removed baseline entries for issues resolved by the refactoring in Client.php
server.pid Process ID file, likely should not be committed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@paulinevos paulinevos force-pushed the 1728-encryption-usage branch from 636e924 to 293fa6f Compare December 19, 2025 14:20
So we can unit test options, as we're transforming some of the options
that are passed
@paulinevos paulinevos force-pushed the 1728-encryption-usage branch from 293fa6f to eef1068 Compare December 19, 2025 14:29
@paulinevos paulinevos force-pushed the 1728-encryption-usage branch from eef1068 to 952cd62 Compare December 19, 2025 14:55
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 paulinevos force-pushed the 1728-encryption-usage branch from 952cd62 to 0213c1e Compare December 19, 2025 14:58
@paulinevos paulinevos enabled auto-merge (squash) December 19, 2025 15:02
@paulinevos paulinevos merged commit bd5ac24 into mongodb:v2.x Dec 19, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants