Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

Change from type to bytes from string #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Dec 19, 2016

The string should be always valid UTF-8 data: see string type in the table https://developers.google.com/protocol-buffers/docs/proto#scalar

We are currently sending binary multihash over it. There are some other mistakes like that in IPFS proto files. There might be more in this file.

@jbenet
Copy link
Contributor

jbenet commented Dec 20, 2016

sgtm. @diasdavid ?

@daviddias
Copy link
Member

Yes! :) The same problem is happening in the Bitswap Message proto -- https://github.com/ipfs/go-ipfs/blob/master/exchange/bitswap/message/pb/message.proto#L8 --, can we get that changed as well? We are already patching it by having a different proto file for js-ipfs.

Shouldn't the Go serializer catch this?

@Kubuxu Will you be able to fix both these issues before 0.4.5? Can you keep me up to date?

@whyrusleeping
Copy link
Contributor

string and bytes are aliases as far as protobuf is concerned. This is simply a problem with the json marshaling code for the commands lib. We need to instruct it on how to marshal binary data

@Kubuxu
Copy link
Member Author

Kubuxu commented Dec 28, 2016

If we adhere to what protobuf recommends (binary data in bytes, valid utf-8 in string) then all our marshalling problems will go away as emitted structure will be []byte and it will get base64 encoded by the native Go JSON marshaller.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants