richard/imageHash

[chomosuke]

speed baby.

Thanks to a certain person's good coding style that he went on to forget about during the meeting.

closes #76.

pre-apologize for any mistake I might have made. pls check.

[waltervan00]

lgtm

[shangzhel]

When I said "use an image hash" I meant that the image URL becomes /api/image/<hash>. The card ID should no longer be used because in the event that more than one card uses the same image, the image hash will be identical, and a single fetch by the browser can cover all cases with its cache.

[chomosuke]

But this does work though? with less changes for the API?

For api/image/hash, we'd have to search the entire card collection, a lot more work for the server in my opinion.

Alternatively image will no longer be associated with card, so they'll have their own collections. And imageHash should probably be replaced by image id. This does mean we no longer search the entire collection for every fetch, but every upload the server will check the hash against every single image in the database. Unsure if this is worth the improvement in UI.

[waltervan00]

I think we should keep the image to card association.
In regards to the api accepting the hash of an image, we could do a query on cards for the hash image and return if found. This could lead to problems regarding multiple cards having the same hash value (a collision may be unlikely but I wouldn't leave it to chance), and in those cases the incorrect image may be retrieved. For this reason I think it would be fine keep the CardId field in the query.

The GET /image endpoint also needs to check the authorisation of whether the user owns the card the image belongs to. This can be done within the transaction code, however it still means that we need to fetch the card record the imagehash falls under.

Currently the improvement is front-end centric and the goal is to ensure that the Image component makes the calls whenever an image is changed and a new url is loaded into the component.

[chomosuke]

I agree with walter.

I'll list the (what i think is the) pros and cons here:

We do what I did here: pretty much nothing changes, hash is only used by the front-end to get browser to behave.

We do /api/image/hash: two options:
options no 1, nothing change on the server except GET image, get image will now scan through every card to find the user/image combination this can potentially be accomplished quickly by creating an index of card with imageHash.

options no 2, image unassociate with cards. Every card will have an imageId with it. Sever scan for identical hash on every upload to save space (or alteratively not, but in that case you might as well keep the association). Server will also still have to check if user can access image, this means image will have a list of user's foreign keys.

[shangzhel]

This could lead to problems regarding multiple cards having the same hash value (a collision may be unlikely but I wouldn't leave it to chance), and in those cases the incorrect image may be retrieved.

You are correct, and the MD5 hash is susceptible to this. Use the createHash('sha256') function from the crypto module to create a Hash object with the SHA-256 hash algorithm. If you are worried about hash collisions with SHA-256, NIST calls it secure in the FIPS 180-4 Secure Hash Standard.

[shangzhel]

We do /api/image/hash: two options:

Neither options are good. There is one easy way to do it. Have every card store its image and the imageHash, and in GET /api/image call await cards.findOne({'user':req.user._id,'imageHash':hash}). If the promise resolves with null, then no such combination exists (either the card belongs to another user, or no card with the hash exists at all, we don't care about why either way) and the request should fail with 404 Not Found. Otherwise such a card image is found, and it is sent as the response.

[chomosuke]

So basically the first option?

[shangzhel]

Almost done

[shangzhel]

One minor change, then it should be good.

[shangzhel]

Looks good to me