Why do we specify @since on an internal API? (the same applies to MongoKeyDecryptor) I fail to see an answer myself.

I see that some existing program elements in CAPI have @since specified, while some other do not (like mongocrypt_setopt_enable_multiple_collinfo, which is relatively new).

Just convention I suppose and I suppose its slightly informative - we don't publish the javadocs either yet write them.

Resolving as its a Nit / question and not worth changing

Ended up removing all @since tags here - as you mentioned the usage was mixed. Also its really tied to the libmongocrypt version.

Let's add the

Retrieve it with @ref mongocrypt_ctx_status

sentence from the documentation in mongocrypt.h to the @return text. It is useful, and we seem to have it in the documentation of other methods. Though it should be formatted properly according to the Java documentation comment rules.

Updated all Javadocs to bring inline with mongocrypt.h as of 1.18.1

• mongocrypt_kms_ctx_feed_with_retry was addded later in MONGOCRYPT-763 add in-place retry API libmongocrypt#1011 (https://jira.mongodb.org/browse/MONGOCRYPT-763).
• The original prose, which JAVA-5391 and this PR are supposed to implement, instructed us to use mongocrypt_kms_ctx_feed. The current prose, added in the above libmongocrypt PR, instructs us to use either mongocrypt_kms_ctx_feed or mongocrypt_kms_ctx_feed_with_retry.

Thus, we do not have to use mongocrypt_kms_ctx_feed_with_retry. If we decide to introduce this method and use it, then we should
also

1. fully implement https://jira.mongodb.org/browse/MONGOCRYPT-763 in this PR (it seems, it mostly was implemented?);
   • example: the documentation of mongocrypt_kms_ctx_fail was updated in MONGOCRYPT-763 add in-place retry API libmongocrypt#1011, and that change is not reflected in the current PR.
2. specify JAVA-5772 in addition to JAVA-5391 in the description of the current PR
3. resolve https://jira.mongodb.org/browse/JAVA-5772 (split from https://jira.mongodb.org/browse/MONGOCRYPT-763) once this PR is merged.

I wonder how the PR ended up using a method from a different change, why nothing was explicitly said about that to help orienting a reviewer, and what was the plan for JAVA-5772 given that this PR implements most of what should be done in JAVA-5772.

Thanks - Let me review JAVA-5772 and ensure all is covered there. I followed integrating.md but that most likely was the latest version and effectively have done both tickets.

Confirmed this includes JAVA-5772. I didn't see a drivers ticket for it and missed it completely. Not that its very clear either.

For some reason the documentation of this method in the PR omits parts of the documentation from mongocrypt.h. Let's bring them all here, unless there is a good reason to omit any specific part.

Updated all javadocs.

Could you please

• Either explain the problem in this GitHub thread, or link to an existing explanation / bug report.
  • Also explain why the fix is required for the current PR.
• Share a link to the upstream PR/commit with the fix.

I think, given that this change is in a vendored code, we should do it in a separate PR, to avoid complicating the future work of updating the vendored code.

Yes it was causing the tests to fail (I can't recall which one) but it was not returning bytes that needed to be read. Most likely the mock kms server.

Ah yes it was:

ClientSideEncryptionKmsRetryProseTest > testCreateDataKeyAndEncryptWithHttpRetry(String) > Case 2: HTTP retry with aws FAILED

If causes: MongoClientException: Exception in encryption library: Allocation size must be greater than zero.

It doesn't happen on sync because it uses SSLSocket/InputStream, which correctly delivers data before signalling EOF on close_notify.

It isn't triggered by gcp or azure because the failpoint fires on the OAuth token request. So the actual key operation uses another connection and succeeds. As AWS doesn't use a token, and fails with data unconsumed on the wire.

According to Claude:

The TLS spec allows close_notify to arrive in the same flight as application data. > Any real-world TLS server can legitimately:

• Send Connection: close and the response in one write (common for HTTP/1.0 or error responses)
• Send a response and immediately initiate TLS shutdown
• Have the OS coalesce the response and close_notify into a single TCP segment due to Nagle's algorithm or buffering

TLSChannelImpl fixed this behaviour in 0.5.0

The current name feedAndRetry suggests that the method also retries, which is not true. Let's find a name that is less misleading. Maybe feedWithRetry (as is in CAPI), or feedWithRetrySupport?

Done

Where does this constant and the rules accompanying it come from? It seem to be in conflict with both the current integration.md instructions and the API documentation of the methods mongocrypt_kms_ctx_bytes_needed, mongocrypt_kms_ctx_feed_with_retry.

As per the comment above - it matches DEFAULT_MAX_KMS_BYTE_REQUEST.

The fail() function resets the parser and sets should_retry = true, which deliberately makes bytes_needed return 0. This creates an awkward contract: the driver must somehow know to do a read even though bytes_needed says "I don't need anything."

The integrating guide doesn't document this gap — it just says "loop while bytes_needed > 0" and "call fail() on network error, retry if it returns true," without acknowledging that those two instructions are incompatible without a fallback.

I asked Claude to compare my implementation to that of the python binding. Seems I over complicated it (or shortcutted it) and I can just revisit the State machine loop. Will update.

Let's make MONGOCRYPT_CTX_NEED_KMS a @link to the corresponding program element.

Updated all javadocs

• Let's update the documentation of the mongocrypt_kms_ctx_bytes_needed method that existed before this PR, such that it mentions not only to the mongocrypt_kms_ctx_feed method, but also to the mongocrypt_kms_ctx_feed_with_retry method.
• Let's express those notions using @link.

Updated all javadocs

True if the request should be retried

The documentation of the native method says "may be retried", not "should", which is different. Let's make our documentation consistent with that of the native API, and update the documentation of MongoKeyDecryptor.fail accordingly.

I've got Claude to update them all - as most are very out of date.

I've also added JAVA-6225 to remove any unused statics / methods that either we never or no longer implement

1. We have @NotThreadSafe for this, let's use it.
   • Also, given that for some non-obvious reason (do you know what it is?) we use MongoKeyDecryptorImpl via the MongoKeyDecryptor interface, we should document the interface as non-thread-safe.
2. If you want to document explicitly that the wrapped mongocrypt_kms_ctx_t is not thread-safe, then let's do that in the documentation of that type, not here.
3. The "methods mutate the underlying..." text is not needed - it does not add anything useful to the "not thread-safe" information, nor does it on its own imply non-thread-safety.
4. "Callers perform retries sequentially ..." - all that information may be a comment on the calling code, if it's really needed there. I think, the documentation of MongoKeyDecryptorImpl is a wrong place for the comments for the code that uses the type.

Opted to remove it. Unfortunately, @NotThreadSafe isn't available to use here.