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

decode_abi with a string returns a bytes value #81

Closed
carver opened this issue Jul 12, 2018 · 7 comments
Closed

decode_abi with a string returns a bytes value #81

carver opened this issue Jul 12, 2018 · 7 comments

Comments

@carver
Copy link
Collaborator

carver commented Jul 12, 2018

  • Version: df3fc70 (master at creation time)
  • Python: 3.6
  • OS: linux

What was wrong?

I would expect that decoding an abi value of string would return a str-typed value.

In [1]: from eth_abi import decode_abi

In [2]: decode_abi(['string'], b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05mystr\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00')
Out[2]: (b'mystr',)

I expected ('mystr',)

How can it be fixed?

Last step of decoding should be to call .decode('utf8') on a bytes value if using a string decoder.

@pipermerriam
Copy link
Member

Noting that this will result in string values that cannot be decoded due to containing invalid UTF8 characters, but I think this is ok since the docs for the string type state that it is assumed to be UTF8 encoded.

@carver
Copy link
Collaborator Author

carver commented Jul 12, 2018

assumed to be UTF8 encoded

Right, although it might be desirable for users to retrieve the bytes data even if it's not valid utf8. (rather than just crash)

Maybe would should use something like .decode("utf-8", "replace"). Although I kind of hate it already, the idea that broken strings won't be noticed early on. I'm just not sure how else to reasonably give people access to malformed data. Maybe we just don't. If you want malformed data, you write a curl call to the json-rpc.

I guess I landed on: crash if not valid utf8.

@pipermerriam
Copy link
Member

How about we raise a custom error message that is as informative as possible when this happens.

The returned type for this function is string which is expected to be a UTF8 encoded string of text. The returned value ... could not be decoded as valid UTF8. This is indicative of a broken application which is using incorrect return types for binary data.

@dylanjw
Copy link
Contributor

dylanjw commented Jul 16, 2018

Im going to work on this because it will simplify the log data filter for web3: ethereum/web3.py#953

Just to recap, to see that Im understanding.

  1. Do return a string type, UTF-8 decoded for abi strings.
  2. If .decode("utf-8") fails raise an exception.

@pipermerriam
Copy link
Member

Correct

@carver
Copy link
Collaborator Author

carver commented Jul 16, 2018

👍 I think this will require a major version bump.

@davesque
Copy link
Contributor

I suppose this is fixed.

pacrob added a commit to pacrob/eth-abi that referenced this issue Apr 14, 2023
…m#81)

* apply template updates found following merge with eth-typing

* add build as a dev dependency

* remove timeout from pytest.ini, it doesn't do anything without pytest-timeout as a dep
pacrob added a commit to pacrob/eth-abi that referenced this issue Dec 9, 2023
…m#81)

* apply template updates found following merge with eth-typing

* add build as a dev dependency

* remove timeout from pytest.ini, it doesn't do anything without pytest-timeout as a dep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants