Correct data setting into Hash object

[spangin]

Description

Fixes #1566
Correct data setting into Hash object:

• source data is cut if it's too long
• 0s are added if source data is too short. I.e. padding for some data (e.g. Tag) can be removed.

Type of change

• Bug fix (a non-breaking change which fixes an issue)
• Enhancement (a non-breaking change which adds functionality)

How Has This Been Tested?

All existing unit-tests are passed.

Checklist:

• My code follows the style guidelines for this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[GalRogozinski]

I think you still need source.length -sourceOffset

Suggested change

	System.arraycopy(source, sourceOffset, dest, 0, Math.min(dest.length, Math.min(source.length, sourceSize)));
	System.arraycopy(source, sourceOffset, dest, 0, Math.min(dest.length, Math.min(source.length - sourceOffset, sourceSize)));

I don't we currently really use this size optimization. But maybe we will so might as well add support.

Would you like to wrap this line in a method with default visibility, add the @VisibleForTesting annotation, and add a unit task.
Since you are fixing a bug, we need to write a unit test so that the bug will never repeat itself.

[spangin]

Hash object is just a storage for data. The constructor parameters (source, sourceOffset and sourceSize) are description of the data source.
Hash object can't validate data. It can only check data syntax: the data array is not null and data coordinates (sourceOffset and sourceSize) are valid for this array. I.e. if syntax is incorrect, the hash object throws the corresponding exception: NullPointerException or ArrayIndexOutOfBoundsException.
If hash object tries to fix incorrect data source (Math.min(source.length - sourceOffset, sourceSize)), it can be a reason of incorrect data using and a reason of a bug that hard to find.
I.e. my point of view: if sorceOffset + sorceSize > source.length, ArrayIndexOutOfBoundsException has to be thrown to inform about the bug. Anyway you take a decision :)

[GalRogozinski]

I agree with you

[GalRogozinski]

You want to add the exception?

[spangin]

In this situation the adding
if (sourceOffset + sourceSize > source.length) throw new ArrayIndexOutOfBoundsException();
to the code is incorrect, because IOTA uses

iri/src/main/java/com/iota/iri/model/HashFactory.java

Line 79 in 9f95ef7

public Hash create(byte[] trits, int sourceOffset) {

and

iri/src/main/java/com/iota/iri/model/HashFactory.java

Line 100 in 9f95ef7

public Hash create(Class<?> modelClass, byte[] source) {

As you can see the methods don't get sourceSize as input parameter and use Hash.SIZE_IN_TRITS or Hash.SIZE_IN_BYTES as values for sourceSize to invoke the corresponding constructor.

BTW, I have checked the constructor with the code
if (sourceOffset + sourceSize > source.length) throw new ArrayIndexOutOfBoundsException();
and got errors for existing unit-tests:

TransactionViewModelTest.findShouldBeSuccessful:339 » ArrayIndexOutOfBounds
TransactionViewModelTest.findShouldReturnNull:352 » ArrayIndexOutOfBounds

As a result I suggest using the code I provided in this PR.

[GalRogozinski]

LGTM