fix(crypto): crc32 algorithm fix

[dariaterekhova-actionengine]

During zip functionality development I found out that our crc32 algorithm returns wrong values. So I made a fix and checked on a several files that hashes now are exactly the same as in bash zip utility.

[ibgreen]

Any references that you can link to in code? More test cases?

[ibgreen]

Why are we changing sha256 test results in a PR that makes changes to crc32?

[dariaterekhova-actionengine]

I checked it with shasum bash utility and the one that had been commented is the correct one for this data. I found out that some of our crypto classes gives the correct result only once after the initialisation. But this test had been used the same class multiple times, so it has given the wrong result on second use and after. So probably somebody wanted just to make this test work...

[ibgreen]

That is a huge bug that we must fix first. We shouldn't work around it!

#2824

[dariaterekhova-actionengine]

Basic Idea of this change is to create a new object from class every time we want to use the algorithm, not to reuse the old one, because I found out that some classes calculate the right result only once after initialisation. And the reason why I decided not to use the name inside the class is because in this case I need to initialise all of them just in order to get the one I need. But if you think that it's the right way - no problem, I can turn it back

[ibgreen]

Nope, fixing in #2824

[ibgreen]

What I don't like with this is that use of the >>> operator is very rare, I am only vaguely familiar with what it does and I don't understand why this is a fix. because of this, it will be hard to maintain.

My interpretation (possibly incorrect) is that shifting 0 bits is essentially a no-op and just ensures that the input is a positive integer. But the old code did calculate an absolute value with Math.abs(), so that was not enough, so what else does this operator do?

Could we find out and be more explicit and say Math.abs(Math.???(...))

[dariaterekhova-actionengine]

The difference there that 1 in bite representation is 00000000000000000000000000000001 and -1 is 11111111111111111111111111111111. So Math.abs() completely changes the bits there, but what we need is just make JS count first 1 bit as a part of a number and not the sign marker. We don't need to change anything else in this bit representation. And that's exactly what >>> does.

We can also implement it like this:

const toUnsignedInt32 = (n: number): number => {
  if (n >= 0) {
    return n;
  }
  return 0xFFFFFFFF - (n * -1) + 1;
}

We perform this operation once per hash generation, so it shouldn't affect performance

[ibgreen]

Thank you, that makes sense, negative numbers are in two's complement. If you could add this comment above the >>>.

The difference there that 1 in bit representation is 00000000000000000000000000000001 and -1 is 11111111111111111111111111111111. So Math.abs() completely changes the bits there, but what we need is just make JS count first 1 bit as a part of a number and not the sign marker. We don't need to change anything else in this bit representation. And that's exactly what >>> does.

[ibgreen]

@dariaterekhova-actionengine I landed the hash reuse fixes. If you rebase on master and remove any code that was working around that issue, this should be good to land.

[ibgreen]

Looks great, thanks for good teamwork!