-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix(kad): Skip invalid multiaddr #3280
Conversation
With this commit `libp2p-kad` no longer discards the whole peer payload in case an addr is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining payload. See libp2p#3244 for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing this!
protocols/kad/CHANGELOG.md
Outdated
- Skip invalid multiaddr in `Peer` `addrs`. See [PR XXX]. | ||
|
||
[PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Skip invalid multiaddr in `Peer` `addrs`. See [PR XXX]. | |
[PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX | |
- Skip invalid multiaddr in `Peer` `addrs`. See [PR 3280]. | |
[PR 3280]: https://github.com/libp2p/rust-libp2p/pull/3280 |
protocols/kad/src/protocol.rs
Outdated
@@ -35,6 +35,7 @@ use futures::prelude::*; | |||
use instant::Instant; | |||
use libp2p_core::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo}; | |||
use libp2p_core::{Multiaddr, PeerId}; | |||
use log::debug; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal preference only but I like not importing these because they often come and go with refactorings and log::debug
is actually very expressive.
Mergify can backport PRs btw: https://docs.mergify.com/actions/backport/ It might interact oddly with our version bumping and changelog entries though. |
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With this commit `libp2p-kad` no longer discards the whole peer payload in case an addr is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining payload. See libp2p#3244 for details. Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
With this commit `libp2p-dcutr` no longer discards the whole remote payload in case an addr is unparsable, but instead logs the failure and skips the unparsable multiaddr. See libp2p#3244 for details.
Description
With this commit
libp2p-kad
no longer discards the whole peer payload in case an addr is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining payload.See #3244 for details.
Notes
Targets the
v0.50
branch.Links to any relevant issues
Open Questions
Change checklist