-
Notifications
You must be signed in to change notification settings - Fork 792
fix: ethers_providers::is_local_endpoint with rust matching pattern on URL::parse #2351
Conversation
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,
would appreciate a smol sanity test for this
You want one more unit test? I thought doctest were enough? |
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.
ah right,
lgtm
ethers-providers/src/rpc/provider.rs
Outdated
if let Some(host) = url.host() { | ||
match host { | ||
Host::Domain("localhost") => return true, | ||
Host::Ipv4(ipv4) => return ipv4 == Ipv4Addr::LOCALHOST || ipv4.is_link_local(), |
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.
What about ipv4.is_private()
and ipv4.is_loopback()
?
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.
Added with enabling also loopback and private addresses
ethers-providers/src/rpc/provider.rs
Outdated
// skipping IPV6 check | ||
_ => return false, |
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.
Any reason not to include the same for Ipv6Addr
? (ipv6.is_loopback()
)
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.
good point loopback is available for both variants
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.
Added with supporting also ipv6
@mattsse @DaniPopes all good? |
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!
Related to #2346 and foundry issue #4658
Motivation
ethers_providers::is_local_endpoint
function would only work withlocalhost
and127.0.0.1
strings.Solution
From substring matching to rust pattern matching based on
URL::parse
result.PR Checklist