From ecf57d1f115ae758676af36a13d4cd9d4c83a41f Mon Sep 17 00:00:00 2001 From: Gautam Singh Date: Sat, 17 Mar 2018 16:29:09 +0530 Subject: [PATCH 1/3] Revamped the check digit calculation inspired by: https://github.com/chilts/node-coupon-code/issues/8 * Added `V` as the replacement for `Y` character (thus appropriately reducing number of symbols) * Made the modulo operation (in `checkDigitAlg1`) to auto adjust to the size of the symbols array and thus including all the symbols in a pseudo random fashion. * Added a concise guideline regarding why not to use 32 (or any 2^x number) as modulo with the link to the issue discussing about the same. --- coupon-code.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/coupon-code.js b/coupon-code.js index bf2d2a2..0ae54c7 100644 --- a/coupon-code.js +++ b/coupon-code.js @@ -25,7 +25,10 @@ var badWordsList = ('SHPX PHAG JNAX JNAT CVFF PBPX FUVG GJNG GVGF SNEG URYY ZHSS 'NEFR FUNT GBFF FYHG GHEQ FYNT PENC CBBC OHGG SRPX OBBO WVFZ WVMM CUNG') .replace(/[a-zA-Z]/g,function(c){return String.fromCharCode((c<="Z"?90:122)>=(c=c.charCodeAt(0)+13)?c:c-26);}) .split(' '); -var symbolsStr = '0123456789ABCDEFGHJKLMNPQRTUVWXY'; + +// Do make sure the number of symbols used is not a multiple of 2 to have better radomness of check digit +// Refer: https://github.com/chilts/node-coupon-code/issues/8 for more information +var symbolsStr = '0123456789ABCDEFGHJKLMNPQRTUVWX'; var symbolsArr = symbolsStr.split(''); var symbolsObj = {}; symbolsArr.forEach(function(c, i) { @@ -83,7 +86,8 @@ module.exports.validate = function(code, opts) { .replace(/O/g, '0') .replace(/I/g, '1') .replace(/Z/g, '2') - .replace(/S/g, '5'); + .replace(/S/g, '5') + .replace(/Y/g, 'V'); // split in the different parts var parts = []; @@ -137,7 +141,7 @@ function checkDigitAlg1(data, check) { check = check * 19 + k; }); - return symbolsArr[ check % 31 ]; + return symbolsArr[ check % symbolsArr.length ]; } function hasBadWord(code) { From 85cae1d4dcc4fa942d01f6fe046e10a5b6e23a31 Mon Sep 17 00:00:00 2001 From: Gautam Singh Date: Sat, 17 Mar 2018 16:58:11 +0530 Subject: [PATCH 2/3] Updated test cases according to the new symbols Since `Y` is being replaced by `V` previous parts containing `Y` would not have same check digit --- test/validate.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/validate.js b/test/validate.js index c2f806c..6eefbf0 100644 --- a/test/validate.js +++ b/test/validate.js @@ -35,7 +35,7 @@ test("Validating Various Inputs", function (t) { t.ok( cc.validate('1K7Q-CTFM', { parts : 2 }), "but accepted with correct 'parts'"); t.ok(!cc.validate('CTFM-1K7Q', { parts : 2 }), "parts must be in correct order"); - t.ok( cc.validate('QBXA5CV4Q85E-HNYV4U3UD69M-B7XU1BHF3FYE-HXT9LD4Q0DAH-U6WMKC1WNF4N-5PCG5C4JF0GL-5DTUNJ40LRB5', { parts : 7, partLen : 12 }), "but accepted with correct 'parts'"); + t.ok( cc.validate('QBXA5CV4Q85E-HNYV4U3UD69R-B7XU1BHF3FYB-HXT9LD4Q0DAH-U6WMKC1WNF4N-5PCG5C4JF0GL-5DTUNJ40LRB5', { parts : 7, partLen : 12 }), "but accepted with correct 'parts'"); t.equal( cc.validate('1k7q-ctfm-lmtc'), '1K7Q-CTFM-LMTC', "lowercase code is fixed and valid"); @@ -65,14 +65,14 @@ test("Validating Various Inputs", function (t) { t.ok( cc.validate('1K7Q-CTFM-LMTC', { parts : 3 }), 'valid code-pretest'); t.ok(!cc.validate('1K7Q-CTFM-LMT1', { parts : 3 }), 'invalid checkdigit rejected in part 3'); - t.ok( cc.validate('7YQH-1FU7-E1HX-0BG9', { parts : 4 }), 'valid code-pretest'); - t.ok(!cc.validate('7YQH-1FU7-E1HX-0BGP', { parts : 4 }), 'invalid checkdigit rejected in part 4'); + t.ok( cc.validate('7YQN-1FU7-E1HX-0BG9', { parts : 4 }), 'valid code-pretest'); + t.ok(!cc.validate('7YQN-1FU7-E1HX-0BGP', { parts : 4 }), 'invalid checkdigit rejected in part 4'); - t.ok( cc.validate('YENH-UPJK-PTE0-20U6-QYME', { parts : 5 }), 'valid code-pretest'); - t.ok(!cc.validate('YENH-UPJK-PTE0-20U6-QYMT', { parts : 5 }), 'invalid checkdigit rejected in part 5'); + t.ok( cc.validate('YENK-UPJK-PTE0-20U6-QYMK', { parts : 5 }), 'valid code-pretest'); + t.ok(!cc.validate('YENK-UPJK-PTE0-20U6-QYMK', { parts : 5 }), 'invalid checkdigit rejected in part 5'); - t.ok( cc.validate('YENH-UPJK-PTE0-20U6-QYME-RBK1', { parts : 6 }), 'valid code-pretest'); - t.ok(!cc.validate('YENH-UPJK-PTE0-20U6-QYME-RBK2', { parts : 6 }), 'invalid checkdigit rejected in part 6'); + t.ok( cc.validate('YENK-UPJK-PTE0-20U6-QYMK-RBK1', { parts : 6 }), 'valid code-pretest'); + t.ok(!cc.validate('YENK-UPJK-PTE0-20U6-QYMK-RBK2', { parts : 6 }), 'invalid checkdigit rejected in part 6'); t.end(); }); From 2288731e842b764377d1ad74be935a567cefa9dc Mon Sep 17 00:00:00 2001 From: Gautam Singh Date: Sat, 17 Mar 2018 17:01:46 +0530 Subject: [PATCH 3/3] False positive case of `Y` containing failure part Mistakenly updated the check digit for a `Y`character containing part that should fail according to the test case scenario --- test/validate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/validate.js b/test/validate.js index 6eefbf0..2138b2f 100644 --- a/test/validate.js +++ b/test/validate.js @@ -69,7 +69,7 @@ test("Validating Various Inputs", function (t) { t.ok(!cc.validate('7YQN-1FU7-E1HX-0BGP', { parts : 4 }), 'invalid checkdigit rejected in part 4'); t.ok( cc.validate('YENK-UPJK-PTE0-20U6-QYMK', { parts : 5 }), 'valid code-pretest'); - t.ok(!cc.validate('YENK-UPJK-PTE0-20U6-QYMK', { parts : 5 }), 'invalid checkdigit rejected in part 5'); + t.ok(!cc.validate('YENK-UPJK-PTE0-20U6-QYMT', { parts : 5 }), 'invalid checkdigit rejected in part 5'); t.ok( cc.validate('YENK-UPJK-PTE0-20U6-QYMK-RBK1', { parts : 6 }), 'valid code-pretest'); t.ok(!cc.validate('YENK-UPJK-PTE0-20U6-QYMK-RBK2', { parts : 6 }), 'invalid checkdigit rejected in part 6');