adlnet / lrs-conformance-test-suite

A NodeJS project that tests the MUST requirements of the xAPI Spec and is based on the ADL testing requirements repository. The test suite website can be found here: https://lrstest.adlnet.gov/. The adopters website can be found here: https://adopters.adlnet.gov/
https://adlnet.gov/projects/xapi/
MIT License
67 stars 42 forks source link

Incorrect hash for: should succeed when attachment is raw data and request content-type is "multipart/mixed" #185

Closed bscSCORM closed 7 years ago

bscSCORM commented 7 years ago

For the test:

should succeed when attachment is raw data and request content-type is "multipart/mixed"

This template file is being used: https://github.com/adlnet/lrs-conformance-test-suite/blob/master/test/v1_0_3/templates/attachments/basic_image_multipart_attachment_valid.part

Previously, I had logged this issue about this test, and it has been addressed: Related to: https://github.com/adlnet/grail-issue-tracker/issues/67

However, by looking at the old and new version of the template file, it appears that the issue was addressed by doing a global search and replace for CR and replacing it with CRLF. In any case, there are now CRLF pairs within the binary attachment portion of the file that used to just be CR, and the hash has not been updated.

So, a couple things here. The hash is no longer correct for the specified content, we calculate it as dddc71880bba0d1537e13a580ddac940a69086e3ac14bf562cfa23d8f02a28b2 upon receiving that attachment, and since we accept other attachments properly (eg: signatures) where the test suite calculates the hash, it seems like our hash is likely to be correct.

It's probably a reasonable short-term fix to update the hash.

Longer term, I'm concerned that the way this test is set up is a bit "brittle", including a hard-coded hash and header in the same file along with the binary attachment itself... it seems like it would be better to set the attachment aside as a binary file, load it, and calculate the hash in the test.