bcgit / pc-dart

Pointy Castle - Dart Derived Bouncy Castle APIs
MIT License
233 stars 122 forks source link

BasePadding to remove padding instead of 'take'ing it #176

Closed CaringDev closed 1 year ago

CaringDev commented 1 year ago

The following test currently fails:

      var data = Uint8List.fromList([1, 2, 3, 3, 3]);
      final pkcs7 = PKCS7Padding()..init();
      final padCount = pkcs7.padCount(data);
      expect(padCount, 3);
      final d = pkcs7.process(false, data);
      // fails, length is 3
      expect(d.length, data.length - padCount);

As far as I can tell, there is no process function in the original Java implementation: see PKCS7Padding and BlockCipherPadding

I'm not sure this is a safe change, as it might negatively affect other paddings.

mwcw commented 1 year ago

Hi,

I think what has happened is that this method was added and lost over time.

Padding is added and removed by PaddedBlockCipher which does not call this method.

Interestingly enough that process method in BasePadding cannot add padding because it is not suppled with a final length to pad up to or a block size for example.

I replaced the body of the method with a throw and it was never called by any of the tests.

If it is being used by anyone I don't know what for to be honest.

I'll merge this in but I have added an exception if pad is true.

Thanks for the submission!

MW