Remove scheme from URI before fetching path for CRL path check #2622
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…gic.java Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
PR Overview
This PR refactors the logic for obtaining a usable file path for CRL path checks by removing the URI scheme from the path before calling Paths.get(). It introduces a more robust way to handle file URIs and adds an appropriate error message for invalid paths.
- Extracts the scheme-specific part from the URI and removes its leading "/" before verifying if the path is a directory.
- Adds handling for InvalidPathException with a corresponding error message update in SQLServerResource.java.
Reviewed Changes
| File | Description |
|---|---|
| src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java | Modifies the retrieval of the code source URI to remove the scheme component and adjusts path handling accordingly. |
| src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java | Adds an error message for invalid paths. |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java:296
- Ensure that schemeSpecificPart is non-empty and starts with '/' before removing the first character to avoid unexpected behavior or exceptions.
schemeSpecificPart = schemeSpecificPart.substring(1); // Remove leading /
src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java:301
- [nitpick] Consider extracting the hardcoded string 'target/classes/' into a constant to improve maintainability and clarity.
location = location.substring(0, location.length() - ("target/classes/").length());
There was a problem hiding this comment.
PR Overview
This PR fixes an issue where a URI scheme interferes with fetching the file path for the CRL path check by stripping the scheme before using Paths.get().
- Remove the URI scheme using getSchemeSpecificPart and a new constant for the forward slash.
- Update error handling and add a new resource error message for invalid paths.
Reviewed Changes
| File | Description |
|---|---|
| src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java | Adjusts path extraction from the CodeSource URI by removing the scheme and handling directory checks |
| src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java | Adds a new error message resource for invalid path exceptions |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java:298
- Removing the leading '/' may yield an incorrect relative path on Unix-based systems where an absolute path is required. Consider verifying that this transformation works correctly on all target platforms.
schemeSpecificPart = schemeSpecificPart.substring(1); // Remove leading /
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2622 +/- ##
============================================
- Coverage 51.54% 51.52% -0.02%
- Complexity 3996 4003 +7
============================================
Files 147 147
Lines 33694 33702 +8
Branches 5630 5631 +1
============================================
Hits 17366 17366
- Misses 13877 13905 +28
+ Partials 2451 2431 -20 ☔ View full report in Codecov by Sentry. |
|
Passes tests with run id = 110147. |
Jeffery-Wasty
requested review from
lilgreenbird,
divang,
Ananya2,
tkyc,
muskan124947 and
machavan
When attempting to get the Path for use in Configurable Retry Logic patch check, this could fail if the scheme was not
file. This is resolved by removing the scheme component from the URI before feeding it intoPaths.get().