Skip to content

fix(java/driver/jni): harden JNI exception helpers#4395

Open
lidavidm wants to merge 1 commit into
apache:mainfrom
lidavidm:jni-exception
Open

fix(java/driver/jni): harden JNI exception helpers#4395
lidavidm wants to merge 1 commit into
apache:mainfrom
lidavidm:jni-exception

Conversation

@lidavidm

Copy link
Copy Markdown
Member

Replace assert() guards in both ThrowJavaException helpers with proper null checks that return early when JNI calls fail. Also fix malformed JNI method descriptor in ThrowJavaException.

Assisted-by: Claude Opus 4.8 noreply@anthropic.com

Replace assert() guards in both ThrowJavaException helpers with proper null
checks that return early when JNI calls fail. Also fix malformed JNI method
descriptor in ThrowJavaException.

When any JNI lookup fails, it leaves a pending exception, so returning early
propagates the correct error back to Java.

Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

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 hardens the Java JNI driver’s exception-throwing helpers by replacing assert()-based guards (which can be compiled out in release builds) with explicit null checks, and it fixes an invalid JNI constructor method descriptor for Java exceptions.

Changes:

  • Replace assert(...) checks in both ThrowJavaException helpers with early-return null checks for FindClass, GetMethodID, NewStringUTF, and NewObject.
  • Fix the malformed JNI method descriptor for the (String) constructor from (Ljava/lang/String)V to (Ljava/lang/String;)V.
  • Add an explicit null check for the resolved AdbcStatusCode static field ID before attempting to fetch it.

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

@lidavidm lidavidm requested a review from amoeba June 15, 2026 02:02
@lidavidm lidavidm marked this pull request as ready for review June 15, 2026 02:03
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.

2 participants