Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add byte array to Values message #142

Open
flippmoke opened this issue Aug 26, 2019 · 5 comments
Open

Add byte array to Values message #142

flippmoke opened this issue Aug 26, 2019 · 5 comments

Comments

@flippmoke
Copy link
Member

We currently do not have a way to properly encode byte arrays as the value of an attribute. Ideally, this would not be an allowed type but I can see in some rare cases where this might be useful. I would like to propose adding bytes as the 8th value message and specifically state that it may be ignored in the decoder.

I know that from an data format standpoint that protobuffers do not have any difference between bytes and strings, however, I have come to realize that in some situations such as the python protobuf libraries it attempts to enforce a text encoding type on the values as they are set via the protobuf library. Additionally, it would be ideal for a client to know if there is a non standard string format that it could be ignored for situations where a bytes field is being used.

Thoughts?

/cc @ericfischer @joto

@joto
Copy link

joto commented Aug 26, 2019

Is this a change you want to make to the v2 spec?

@e-n-f
Copy link
Contributor

e-n-f commented Aug 26, 2019

I think I had argued for custom attribute types that would include a MIME type or com.whatever-style identifier as well as the blob, but I agree, it is useful to distinguish strings of text from binary blobs.

My only objection to adding this is that we are still in the same trap we were in before, where too many libraries will throw exceptions when they find unexpected messages in the protobuf instead of cleanly passing them through. Until that is solved somehow, it will be safer to encode binary data as base64 or something similar than to use a new type.

@flippmoke
Copy link
Member Author

@joto no to the v3 spec

@joto
Copy link

joto commented Aug 26, 2019

@flippmoke Shouldn't we then leave the Values alone and backwards compatible and only allow the "binary type" as a new type in the stuctured value encoding?

@flippmoke
Copy link
Member Author

@joto I went brain dead on the solution here this morning -- and yes we should.

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

No branches or pull requests

3 participants