Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

chore: use data-encoding arg to ensure data is not corrupted #806

Merged
merged 1 commit into from
Aug 14, 2018
Merged

chore: use data-encoding arg to ensure data is not corrupted #806

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/object/get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,17 @@ module.exports = (send) => {

send({
path: 'object/get',
args: cidB58Str
args: cidB58Str,
qs: {
'data-encoding': 'base64'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanshaw quick question: what values are allowed here?
Ones from https://github.com/multiformats/multibase/blob/master/multibase.csv ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment go-ipfs supports text and base64.

Go's text is the default (e.g. raw bytes), though go-ipfs doesn't escape the characters so it's http api can produce strings that are invalid in JSON and hard to replicate in JavaScript so base64 is safer.

Once ipfs/js-ipfs#1420 is in, js-ipfs will support whatever Buffer.toString([encoding[, start[, end]]]) does.

If we want to add all of the different encodings from that csv it should be pretty straightforward once both implementations support the data-encoding argument.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @achingbrain!

This PR reminded me of yEnc from Usenet days, so I was just looking at efficiency column in encoding standards comparison and things like Ascii85 (used in PDFs and git patch) looked interesting (due to moderate adoption and smaller overhead, eg. 25% instead of 33%).

..but then i remembered that HTTP API is often gzipped which means any gains introduced by more exotic encoding than Base64 go away.

For example: base-64 seems to compress better than base-122 🙃

}
}, (err, result) => {
if (err) {
return callback(err)
}

result.Data = Buffer.from(result.Data, 'base64')

const links = result.Links.map((l) => {
return new DAGLink(l.Name, l.Size, Buffer.from(bs58.decode(l.Hash)))
})
Expand Down