Skip to content
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

NAS-133655 / Fix app portal URI normalization for IPv6 URIs #15431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mortie
Copy link

@mortie mortie commented Jan 19, 2025

This fixes a bug I reported here: https://ixsystems.atlassian.net/browse/NAS-133655

There's a more thorough investigation there, but basically, when accessing TrueNAS over IPv6, the "Web UI" button in the application detals pane would be broken. This is because it would try to parse http://<IPv6 address>:<port> as a URL, which doesn't work. The front-end gets the URL from the function I modified here.

@mortie mortie changed the title Fix app portal URI normalization for IPv6 URIs NAS-133655 / Fix app portal URI normalization for IPv6 URIs Jan 19, 2025
@yocalebo
Copy link
Contributor

@mortie thanks for the PR! We haven't forgot about it, just trying to wrap up a few other things before we freeze for Fangtooth BETA. Someone should be able to review this soon.

@mortie
Copy link
Author

mortie commented Jan 21, 2025

No worries :) I'm in no rush

# We already assume that host_ip doesn't contain a port number,
# so checking if it contains a ':' should be sufficient to determine
# whether or not it's an IPv6 address.
if ':' in host_ip:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mortie we have added changes where apps now will already have the portal URI normalized if they are ipv6 based (

).

However people who are using apps which were installed/modified before they come to this version, they will run into this issue. Based off this, we definitely would not want to have this normalized 2 times so we should check that [ is not already present and : is present and then normalize it

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.

3 participants