Skip to content

Backport to 9.0 HV-2117 NIP validation incorrectly accepts values ending in 0 #1651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,13 @@ public void initialize(NIP constraintAnnotation) {
protected int[] getWeights(List<Integer> digits) {
return WEIGHTS_NIP;
}

@Override
protected boolean checkTwoDigitModuloResult(char checkDigit) {
// From https://pl.wikipedia.org/wiki/Numer_identyfikacji_podatkowej:
// > NIP jest tak generowany, aby nigdy w wyniku tego dzielenia, jako reszta, nie uzyskać liczby 10
//
// which means that the way NIP is generated the checksum can never be 10, so if we got here, we've got an invalid NIP:
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import org.hibernate.validator.internal.util.ModUtil;

/**
* A base class validator for different Polish identification numbers. They differs in the lengths and weights used to calculate the mod sum.
* A base class validator for different Polish identification numbers. They differ in the lengths and weights used to calculate the mod sum.
* In order to implement one you need to provide a method that gives an array of weights
* and {@link ConstraintValidator#initialize(Annotation)} methods.
*
Expand All @@ -38,11 +38,15 @@ public boolean isCheckDigitValid(List<Integer> digits, char checkDigit) {
switch ( modResult ) {
case 10:
case 11:
return checkDigit == '0';
return checkTwoDigitModuloResult( checkDigit );
default:
return Character.isDigit( checkDigit ) && modResult == extractDigit( checkDigit );
}
}

protected boolean checkTwoDigitModuloResult(char checkDigit) {
return checkDigit == '0';
}

protected abstract int[] getWeights(List<Integer> digits);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.hibernate.validator.constraints.pl.NIP;
import org.hibernate.validator.test.constraints.annotations.AbstractConstrainedTest;

import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

/**
Expand Down Expand Up @@ -40,62 +41,51 @@ public void testIncorrectLength() {
);
}

@Test
public void testCorrectNipNumber() {
assertNoViolations( validator.validate( new Person( "5931423811" ) ) );
assertNoViolations( validator.validate( new Person( "2596048500" ) ) );
assertNoViolations( validator.validate( new Person( "4163450312" ) ) );
assertNoViolations( validator.validate( new Person( "1786052059" ) ) );
assertNoViolations( validator.validate( new Person( "6660057854" ) ) );
assertNoViolations( validator.validate( new Person( "4219220786" ) ) );
assertNoViolations( validator.validate( new Person( "3497264632" ) ) );

@Test(dataProvider = "validNips")
public void testCorrectNipNumber(String nip) {
assertNoViolations( validator.validate( new Person( nip ) ) );
}

@Test
public void testIncorrectNipNumber() {
assertThat( validator.validate( new Person( "123-456-78-14" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "123-45-67-812" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "123-456-32-12" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "5931423812" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "2596048505" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "4163450311" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "1786052053" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "6660057852" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "4219220785" ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
assertThat( validator.validate( new Person( "3497264639" ) ) )
@Test(dataProvider = "invalidNips")
public void testIncorrectNipNumber(String nip) {
assertThat( validator.validate( new Person( nip ) ) )
.containsOnlyViolations(
violationOf( NIP.class ).withProperty( "nip" )
);
}

@DataProvider(name = "validNips")
private static Object[][] validNips() {
return new Object[][] {
{ "5931423811" },
{ "2596048500" },
{ "4163450312" },
{ "1786052059" },
{ "6660057854" },
{ "4219220786" },
{ "3497264632" }
};
}

@DataProvider(name = "invalidNips")
private static Object[][] invalidNips() {
return new Object[][] {
{ "123-456-78-14" },
{ "123-45-67-812" },
{ "123-456-32-12" },
{ "5931423812" },
{ "2596048505" },
{ "4163450311" },
{ "1786052053" },
{ "6660057852" },
{ "4219220785" },
{ "3497264639" },
{ "4062321040" },
{ "7985097620" },
{ "8808817210" }
};
}

public static class Person {

@NIP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public void testCorrectRegon9Number() {
assertNoViolations( validator.validate( new Company( "737024234" ) ) );
assertNoViolations( validator.validate( new Company( "074635672" ) ) );
assertNoViolations( validator.validate( new Company( "593908869" ) ) );
assertNoViolations( validator.validate( new Company( "501827370" ) ) );
}

@Test
Expand Down