Skip to content

Validate reflective class names for connection properties - #3004

Merged
Divang Sharma (divang) merged 6 commits into
mainfrom
user/divang/fix-class-name-validation
Aug 17, 2026
Merged

Validate reflective class names for connection properties#3004
Divang Sharma (divang) merged 6 commits into
mainfrom
user/divang/fix-class-name-validation

Conversation

@divang

Copy link
Copy Markdown
Contributor

Description:
This change adds early validation for reflective class-name values supplied through connection properties such as socketFactoryClass and accessTokenCallbackClass. Malformed values are now rejected before reflective loading, which prevents confusing downstream failures and improves security posture.

What changed
Added validation in Util.newInstance to ensure the supplied value is a valid Java binary class name.
Added a dedicated driver error message for invalid reflective class names.
Added regression tests covering malformed socketFactoryClass and accessTokenCallbackClass values.

Why
Previously, malformed class-name values could reach reflective loading and fail later with less informative errors. This patch fails fast and makes the behavior explicit.

Validation
Verified with Maven compile
Verified with targeted regression tests: 2 tests run

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.23%. Comparing base (a45b6cb) to head (08e426f).

Files with missing lines Patch % Lines
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3004      +/-   ##
============================================
+ Coverage     60.18%   60.23%   +0.05%     
- Complexity     5138     5164      +26     
============================================
  Files           153      153              
  Lines         36673    36679       +6     
  Branches       6733     6733              
============================================
+ Hits          22073    22095      +22     
- Misses        10751    10759       +8     
+ Partials       3849     3825      -24     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@divang Divang Sharma (divang) self-assigned this Aug 4, 2026
@divang Divang Sharma (divang) added the Enhancement An enhancement to the driver. Lower priority than bugs. label Aug 4, 2026
@divang Divang Sharma (divang) added this to the 13.6.0 milestone Aug 4, 2026
@divang
Divang Sharma (divang) requested a lite review from Copilot August 4, 2026 07:54

Copilot AI left a comment

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.

Pull request overview

Adds early validation for connection-property class names that are instantiated reflectively (e.g., socket factory and access token callback), failing fast on malformed values with a dedicated error message to avoid confusing reflective-loading failures.

Changes:

  • Validate className in Util.newInstance using a Java identifier–based binary-name check before calling Class.forName.
  • Introduce a new driver resource string (R_invalidClassNameForProperty) used when rejecting invalid class names.
  • Add regression tests ensuring invalid reflective class-name payloads are rejected early for socketFactoryClass and accessTokenCallbackClass.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/main/java/com/microsoft/sqlserver/jdbc/Util.java Adds pre-reflection class-name validation and new invalid-name error path.
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java Adds the localized error string for invalid reflective class-name values.
src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Adds tests asserting Util.newInstance rejects malformed class-name inputs.

Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/Util.java Outdated
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/Util.java
Comment thread src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated
Use whole-name regex validation, restore the defining class loader, and expand regression coverage for invalid names, trust managers, and assignability.
@muskan124947

Copy link
Copy Markdown
Contributor

testUtilNewInstanceRejectsInvalidSocketFactoryClassName and testUtilNewInstanceRejectsInvalidAccessTokenCallbackClassName are added to SQLServerConnectionTest, which extends AbstractTest and requires SQL Server test config — yet they only exercise Util.newInstance. They also duplicate the UtilTest parameterized coverage. Move them to UtilTest (no DB required) or delete them.

Restore invalid socket factory and access token callback class-name regressions in UtilTest, and add positive coverage for loadable class names.
@divang
Divang Sharma (divang) merged commit 34285fd into main Aug 17, 2026
23 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Closed/Merged PRs in MSSQL JDBC Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement An enhancement to the driver. Lower priority than bugs.

Projects

Status: Closed/Merged PRs

Development

Successfully merging this pull request may close these issues.

5 participants