Skip to content

Throw exception for "Account Locked" SQL Exception 18486#522

Merged
cheenamalhotra merged 2 commits intomicrosoft:devfrom
cheenamalhotra:throwAccountLockedEx
Nov 15, 2017
Merged

Throw exception for "Account Locked" SQL Exception 18486#522
cheenamalhotra merged 2 commits intomicrosoft:devfrom
cheenamalhotra:throwAccountLockedEx

Conversation

@cheenamalhotra
Copy link
Copy Markdown
Member

@cheenamalhotra cheenamalhotra commented Oct 6, 2017

This PR fixes customer issue #483 by throwing exception if Account locked Exception 18486 found.

@cheenamalhotra cheenamalhotra mentioned this pull request Oct 6, 2017
Closed
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 6, 2017 •
edited
Loading

Codecov Report

Merging #522 into dev will decrease coverage by 0.19%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##                dev     #522     +/-   ##
===========================================
- Coverage     46.68%   46.48%   -0.2%     
+ Complexity     2223     2209     -14     
===========================================
  Files           108      108             
  Lines         25312    25313      +1     
  Branches       4181     4182      +1     
===========================================
- Hits          11816    11767     -49     
- Misses        11470    11521     +51     
+ Partials       2026     2025      -1
Flag Coverage Δ Complexity Δ
#JDBC41 46.24% <0%> (-0.13%) 2201 <0> (-9)
#JDBC42 46.36% <0%> (-0.23%) 2206 <0> (-14)
Impacted Files Coverage Δ Complexity Δ
...m/microsoft/sqlserver/jdbc/SQLServerException.java 76.85% <ø> (ø) 29 <0> (ø) ⬇️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 46% <0%> (-0.02%) 277 <0> (ø)
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 51.11% <0%> (-1.49%) 11% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.36% <0%> (-1.13%) 104% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 44.94% <0%> (-1.13%) 16% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.37% <0%> (-0.78%) 238% <0%> (-7%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 54.01% <0%> (-0.57%) 0% <0%> (ø)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 59.17% <0%> (-0.44%) 88% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.58% <0%> (-0.2%) 238% <0%> (-2%)
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 77.62% <0%> (-0.17%) 4% <0%> (-1%)
... and 3 more

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 b00b377...4a1409a. Read the comment docs.

@sandeepchinnari
Copy link
Copy Markdown

sandeepchinnari commented Oct 10, 2017

@cheenamalhotra Thanks for the fix. But this will come in handy when its already too late.
What happens when the password change is in progress and concurrent threads attempt to make connections?

@cheenamalhotra
Copy link
Copy Markdown
Member Author

cheenamalhotra commented Oct 10, 2017 •
edited
Loading

@sandeepchinnari It would be interesting to see how one can reproduce this usecase.

As far as I understand, while request is made to Server to change password, old password is still valid. And password change is an atomic operation to update sys.sql_logins table of database with new password. After which, any new attempt will fail with an error code of 18456 (Logon Failed).

I don't see a reason, why there would be a recurrence of attempts because we are talking nanoseconds here. Even if there is a race condition, as soon as new password is updated, the attempt to connect will fail.

@cheenamalhotra
Copy link
Copy Markdown
Member Author

cheenamalhotra commented Oct 31, 2017

Hi @sandeepchinnari ! Would appreciate your confirmation, if this PR fixes your failed use-case. We would like to merge the PR soon.

@cheenamalhotra cheenamalhotra added this to the 6.3.5 milestone Nov 14, 2017
@cheenamalhotra cheenamalhotra merged commit be14e88 into microsoft:dev Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiangyushawn xiangyushawn Awaiting requested review from xiangyushawn

1 more reviewer

@AfsanehR-zz AfsanehR-zz AfsanehR-zz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

6.3.5

Development

Successfully merging this pull request may close these issues.

5 participants

@cheenamalhotra @codecov-io @sandeepchinnari @AfsanehR-zz @lilgreenbird