Skip to content

Fix for issue when attempting to insert an empty or null value on encrypted column#632

Merged
ulvii merged 7 commits intomicrosoft:devfrom
ulvii:issue624
Mar 7, 2018
Merged

Fix for issue when attempting to insert an empty or null value on encrypted column#632
ulvii merged 7 commits intomicrosoft:devfrom
ulvii:issue624

Conversation

@ulvii
Copy link
Copy Markdown
Contributor

@ulvii ulvii commented Feb 21, 2018 •
edited
Loading

Workaround for #624.
Currently, when inserting empty string or null into encrypted variable-length columns, the driver sets the data length to 0 and the type to type(0) (varchar(0) for example). The workaround is to set the length to 1 and the type to type(1).

@ulvii ulvii mentioned this pull request Feb 21, 2018
Closed
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 21, 2018 •
edited
Loading

Codecov Report

Merging #632 into dev will increase coverage by 0.06%.
The diff coverage is 62.5%.

Impacted file tree graph

@@             Coverage Diff             @@
##               dev     #632      +/-   ##
===========================================
+ Coverage     46.5%   46.57%   +0.06%     
- Complexity    2227     2231       +4     
===========================================
  Files          109      109              
  Lines        25431    25439       +8     
  Branches      4190     4190              
===========================================
+ Hits         11827    11848      +21     
+ Misses       11566    11546      -20     
- Partials      2038     2045       +7
Flag Coverage Δ Complexity Δ
#JDBC42 46.48% <62.5%> (+0.16%) 2224 <0> (+5) ⬆️
#JDBC43 46.31% <62.5%> (-0.04%) 2224 <0> (+5)
Impacted Files Coverage Δ Complexity Δ
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 62.44% <62.5%> (ø) 63 <0> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 44.94% <0%> (-3.38%) 16% <0%> (-2%)
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 51.11% <0%> (-1.49%) 11% <0%> (-1%)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.91% <0%> (-0.66%) 88% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.84% <0%> (-0.13%) 0% <0%> (ø)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.8% <0%> (+0.19%) 241% <0%> (+1%) ⬆️
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 54.4% <0%> (+0.19%) 0% <0%> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 33.36% <0%> (+0.32%) 247% <0%> (+5%) ⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 45.59% <0%> (+2.03%) 105% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db0f41a...688c027. Read the comment docs.

@peterbae peterbae self-requested a review February 23, 2018 00:58
@peterbae
Copy link
Copy Markdown
Contributor

peterbae commented Feb 23, 2018

The workaround looks fine. After looking around the code, it seems like the valueLength is 0 if the user is using a callable statement and using the parameter as an output parameter. Have you considered this case / looked into what giving n/varchar(1) to the server implies?

@peterbae
Copy link
Copy Markdown
Contributor

peterbae commented Feb 23, 2018

I think it should be fine since I just tried the same scenario with output parameter, and it seems to work fine, but I wanted you to consider this scenario as well.

@ulvii
Copy link
Copy Markdown
Contributor Author

ulvii commented Feb 26, 2018

I tested the PR with both PreparedStatement and CallableStatement.

@rene-ye rene-ye added this to the 6.5.0 milestone Feb 27, 2018
@cheenamalhotra cheenamalhotra changed the title Fix for Issue624 Fix for issue when attempting to insert an empty or null value on encrypted column Mar 5, 2018
@ulvii ulvii merged commit f0df377 into microsoft:dev Mar 7, 2018
@ulvii ulvii deleted the issue624 branch March 7, 2018 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

+1 more reviewer

@peterbae peterbae peterbae approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

6.5.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ulvii @codecov-io @peterbae @cheenamalhotra @rene-ye