-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[fuchsia_ctl] Fix IPv6 URL formatting for amber_ctl #46
Conversation
If the local address of the interface that's connected to the Fuchsia device has an IPv6 address the URL to the Amber config.json was formatted incorrectly.
/// | ||
/// Calling this before calling [serveRepo] will result in a [StateError]. | ||
String sourceUrl(String localIp) { | ||
if (localIp.contains(':')) { |
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.
Did we validated this fixes the problem locally?
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.
Nope. And actually, I think it's wrong...
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.
Maybe try https://api.dartlang.org/stable/2.6.1/dart-io/InternetAddress-class.html and get the type from it?
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.
I tried that but InternetAddress fails to parse IPv6 link local addresses. Additionally we need the link local address from the perspective of the Fuchsia device, not the host. I'm working on an update to this PR that actually does that.
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.
When I ran locally " amberctl add_src" I just added the ipv6 + interface name from the host which is connected to the devices network and it worked. The device was able to access the amber server.
Devices in the lab are connected to a similar network setup " a nuc(host) with two interfaces one for internet access/lab network access and another one for internal nat network. The device is connected to the nat network. I think eno2 should work
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.
It may work but I think it's incorrect. With the second commit in this PR I've made the logic match that in fx add-update-source
.
I think this is right now. PTAL. |
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 adding the fix!
If the local address of the interface that's connected to the Fuchsia
device has an IPv6 address the URL to the Amber config.json was
formatted incorrectly.