xmtp / didethresolver

XMTP Registry Resolver
MIT License
3 stars 1 forks source link

47: add support for handling name bytes if they have nul #50

Closed jac18281828 closed 7 months ago

jac18281828 commented 8 months ago

closes #47

codecov[bot] commented 8 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (bb2c7d6) 97.25% compared to head (fb93d06) 97.24%.

Files Patch % Lines
lib/src/types/ethr.rs 94.66% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #50 +/- ## ========================================== - Coverage 97.25% 97.24% -0.01% ========================================== Files 13 13 Lines 2953 3054 +101 ========================================== + Hits 2872 2970 +98 - Misses 81 84 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

insipx commented 8 months ago

Thanks for the docs updates!

Is it possible that this could also stem from https://github.com/xmtp/didethresolver/pull/49?

Maybe worth a try resolving on that branch. I'm a little apprehensive towards the unsafe{} but will have to take a closer look tomorrow

-- what did the issue actually turn out to be, is to_string_lossy handling null bytes incorrectly?

jac18281828 commented 8 months ago

Thanks for the docs updates!

Is it possible that this could also stem from #49?

Maybe worth a try resolving on that branch. I'm a little apprehensive towards the unsafe{} but will have to take a closer look tomorrow

-- what did the issue actually turn out to be, is to_string_lossy handling null bytes incorrectly?

Yes, if it’s any consolation I felt the same way about unsafe but copied it directly from the example here so I thought it may be proper:

https://doc.rust-lang.org/std/ffi/struct.CStr.html

The nul bytes evidently confuse the parser. Perhaps they do not match the character class, anyway we should filter them somewhere.

We can discuss both prs tomorrow.

insipx commented 7 months ago

nice, odd bug glad you found it