TritonDataCenter / node-triton

Triton client tool and node.js library
57 stars 39 forks source link

TOOLS-2563 node-triton throws "BadDigestError: Content-MD5" when body has non-ascii characters #333

Closed bahamat closed 1 year ago

danmcd commented 1 year ago

So the heart of this change is:

-contentMd5Hash.update(chunk.toString('utf8'), 'binary');
+contentMd5Hash.update(chunk.toString('utf8'));

What I don't understand is why dropping 'binary' makes all the difference. Is that something a JS person Just Knows from contentMd5Hash objects? And the whys of this fix should be in the ticket as analysis.

Answer that and I'll gladly +1.

bahamat commented 1 year ago

What I don't understand is why dropping 'binary' makes all the difference. Is that something a JS person Just Knows from contentMd5Hash objects? And the whys of this fix should be in the ticket as analysis.

We discussed over chat, but for posterity I'll provide a little context here.

This originally started life as TRITON-364 and MANTA-3679. In the transition from node 4 (and prior) to node 6 (and later) the result of these operations changed. The API for using them remained the same. But their behavior changed. Thus, when client/server were on opposite sides of that barrier a hack needed to be put in to be compatible with the "foreign" version. When both client and server are on the same side of that barrier, those hacks will break things.

node-triton moved past node-6 quite a long time ago, because we can't really control what version of node people are using in the wild. So this hack was put in to be compatible with cloudapi still on node-4. When cloudapi was moved to node-6 in TRITON-482, this but was again a lingering issue.

Fortunately (or unfortunately?) this didn't affect 7-bit characters, so any user with only ASCII would be unaffected, which made this go unnoticed for quite some time.