Skip to content

Validate bytes when parsing string#116

Merged
Stebalien merged 1 commit into
multiformats:masterfrom
aratz-lasa:patch-1
Dec 9, 2019
Merged

Validate bytes when parsing string#116
Stebalien merged 1 commit into
multiformats:masterfrom
aratz-lasa:patch-1

Conversation

@aratz-lasa

@aratz-lasa aratz-lasa commented Dec 9, 2019

Copy link
Copy Markdown

If no validation is done, it can create invalid Multiaddr.
Probably there is an obvious reason for not doing it. So, if it is the case, I am sorry.
As an example, you can create an ip6zone Multiaddr from the following string: ip6zone/. However, if you validate it, it raises an error, because it contains /

@Stebalien

Copy link
Copy Markdown
Member

ip6zoneStB should validate the string internally. The ValidateBytes function is meant for validating bytes when parsing a multiaddr that's already in the bytes representation.

Could you change the patch to call the ip6zoneVal function from ip6zoneStB?

(we should also document this)

1 similar comment
@Stebalien

Copy link
Copy Markdown
Member

ip6zoneStB should validate the string internally. The ValidateBytes function is meant for validating bytes when parsing a multiaddr that's already in the bytes representation.

Could you change the patch to call the ip6zoneVal function from ip6zoneStB?

(we should also document this)

@aratz-lasa

Copy link
Copy Markdown
Author

Okay, understood. Maybe it is better to call directly strings.Contains(s, "/")?
Where should we document it?

@Stebalien

Stebalien commented Dec 9, 2019 via email

Copy link
Copy Markdown
Member

@aratz-lasa aratz-lasa force-pushed the patch-1 branch 2 times, most recently from 66e4307 to b00b4ac Compare December 9, 2019 21:00
@aratz-lasa

Copy link
Copy Markdown
Author

Okay, done. I hope it is good enough, tell me if you want to change something.

@Stebalien

Copy link
Copy Markdown
Member

Thanks!

@Stebalien Stebalien merged commit 8f38850 into multiformats:master Dec 9, 2019
Documented Transcoder functions, and fixed ip6zone string validation
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

Successfully merging this pull request may close these issues.

2 participants