Conversation
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
github-project-automation
bot
moved this to New
in API Mediation Layer Backlog Management
github-actions
bot
added
the
Sensitive
There was a problem hiding this comment.
Pull Request Overview
This PR implements IPv6 host support across the APIML codebase by replacing manual URL string formatting with a centralized utility class to properly handle IPv6 addresses. IPv6 addresses require square brackets in URLs to be RFC-compliant.
- Added
UrlUtilsutility class with methods to format IPv6 addresses correctly - Updated URL construction throughout the codebase to use the new utility methods
- Added proper IPv6 address handling in route definitions and service registrations
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| common-service-core/src/main/java/org/zowe/apiml/util/UrlUtils.java | New utility class with IPv6-aware URL formatting methods |
| zaas-service/src/main/java/org/zowe/apiml/zaas/cache/CachingServiceClient.java | Replaced manual URL formatting with UrlUtils.getUrl() |
| security-service-client-spring/src/main/java/org/zowe/apiml/security/client/service/GatewaySecurityService.java | Updated login, query, and OIDC validation endpoints to use UrlUtils |
| gateway-service/src/main/java/org/zowe/apiml/gateway/services/ServicesInfoService.java | Updated base URL construction for service info |
| gateway-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java | Added comprehensive IPv6 handling for route definitions and external URLs |
| gateway-service/src/main/java/org/zowe/apiml/gateway/service/GatewayIndexService.java | Updated WebClient base URL construction |
| gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ZaasSchemeTransformRest.java | Added hostname formatting for ZAAS requests |
| gateway-service/src/main/java/org/zowe/apiml/gateway/config/RegistryConfig.java | Updated gateway service address configuration for IPv6 |
| gateway-service/src/main/java/org/zowe/apiml/gateway/caching/CachingServiceClientRest.java | Updated caching service URL construction |
| api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV3Service.java | Updated OpenAPI server URL formatting |
| api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/ApiDocRetrievalServiceLocal.java | Updated SpringDoc server URL construction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Show resolved
Hide resolved
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Outdated
Show resolved
Hide resolved
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Outdated
Show resolved
Hide resolved
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Outdated
Show resolved
Hide resolved
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Outdated
Show resolved
Hide resolved
...ay-service/src/main/java/org/zowe/apiml/gateway/service/routing/RouteDefinitionProducer.java
Outdated
Show resolved
Hide resolved
common-service-core/src/main/java/org/zowe/apiml/util/UrlUtils.java
Outdated
Show resolved
Hide resolved
EvaJavornicka
moved this from New
to In Progress
in API Mediation Layer Backlog Management
| openAPI.getServers() | ||
| .forEach(server -> server.setUrl( | ||
| String.format("%s://%s/%s", scheme, getHostname(), server.getUrl()))); | ||
| UrlUtils.getUrl(scheme, UrlUtils.formatHostnameForUrl(getHostname())) + "/" + server.getUrl())); |
There was a problem hiding this comment.
getHostname() returns host+port, which contains ":" even if there is no IPv6 address. This will probably create an incorrect URL
There was a problem hiding this comment.
I've considered the possibility that getHostname() might be returning the host along with the port, and I've added handling for that here. But, I suspect the issue might be related to the formatHostnameForUrl() method. I'll take a closer look to better understand what's going wrong.
| } | ||
|
|
||
| // Check if this is an IPv6 address (contains multiple colons) | ||
| if (hostname.contains(":") && !hostname.startsWith("[")) { |
There was a problem hiding this comment.
is it safe to assume that any address containing ":" is IPv6? Isn't there a better validation?
There was a problem hiding this comment.
Yes, that’s a valid point. I also feel that relying solely on ":" for validation might not be sufficient. I’ll explore better alternatives and see what could work more reliably.
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
…host-formatting' into hrishikesh-nalawade/GH4318/IPv6-host-formatting
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
…host-formatting' into hrishikesh-nalawade/GH4318/IPv6-host-formatting
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
|
|
||
| // Check for hostname:port format | ||
| int lastColonIndex = hostname.lastIndexOf(':'); | ||
| if (lastColonIndex > -1) { | ||
| String possibleHost = hostname.substring(0, lastColonIndex); | ||
| String possiblePort = hostname.substring(lastColonIndex + 1); | ||
|
|
||
| // Check if what follows the last colon is a valid port number | ||
| if (isValidPort(possiblePort)) { | ||
| // If we have a port, check if the host part is IPv6 | ||
| if (isIPv6Address(possibleHost)) { | ||
| return "[" + possibleHost + "]:" + possiblePort; | ||
| } | ||
| // If the full string is NOT IPv6, return as-is | ||
| if (!isIPv6Address(hostname)) { | ||
| return hostname; // Regular hostname:port or IPv4:port | ||
| } | ||
| // Otherwise, fall through to check if full hostname is IPv6 | ||
| } | ||
| } | ||
|
|
||
| // No port number, check if it's a plain IPv6 address | ||
| if (isIPv6Address(hostname)) { | ||
| return "[" + hostname + "]"; | ||
| } | ||
|
|
There was a problem hiding this comment.
| // Check for hostname:port format | |
| int lastColonIndex = hostname.lastIndexOf(':'); | |
| if (lastColonIndex > -1) { | |
| String possibleHost = hostname.substring(0, lastColonIndex); | |
| String possiblePort = hostname.substring(lastColonIndex + 1); | |
| // Check if what follows the last colon is a valid port number | |
| if (isValidPort(possiblePort)) { | |
| // If we have a port, check if the host part is IPv6 | |
| if (isIPv6Address(possibleHost)) { | |
| return "[" + possibleHost + "]:" + possiblePort; | |
| } | |
| // If the full string is NOT IPv6, return as-is | |
| if (!isIPv6Address(hostname)) { | |
| return hostname; // Regular hostname:port or IPv4:port | |
| } | |
| // Otherwise, fall through to check if full hostname is IPv6 | |
| } | |
| } | |
| // No port number, check if it's a plain IPv6 address | |
| if (isIPv6Address(hostname)) { | |
| return "[" + hostname + "]"; | |
| } | |
| // SECURITY: Check if the entire string is a valid IPv6 address first | |
| // This prevents incorrectly splitting IPv6 addresses that might have | |
| // numeric suffixes that look like ports | |
| if (isIPv6Address(hostname)) { | |
| return "[" + hostname + "]"; | |
| } | |
| // Check for hostname:port format | |
| // Only split if we're sure it's not an IPv6 address | |
| int lastColonIndex = hostname.lastIndexOf(':'); | |
| if (lastColonIndex > -1) { | |
| String possibleHost = hostname.substring(0, lastColonIndex); | |
| String possiblePort = hostname.substring(lastColonIndex + 1); | |
| // Check if what follows the last colon is a valid port number | |
| if (isValidPort(possiblePort)) { | |
| // SECURITY: Verify the host part is NOT an IPv6 address | |
| // If it is, we need to bracket just the IPv6 part, not the entire string | |
| if (isIPv6Address(possibleHost)) { | |
| // This is an unbracketed IPv6 address with port - format correctly | |
| return "[" + possibleHost + "]:" + possiblePort; | |
| } | |
| // If the host part is not IPv6, it's safe to treat as hostname:port | |
| return hostname; // Regular hostname:port or IPv4:port | |
| } | |
| } |
It is better to check for IPv6 address before looking for port number
There was a problem hiding this comment.
Hello @achmelo ,
The problem with checking the entire string as IPv6 before is that, in case where port is present with host (example: - 2001:db8::1:8080), The InetAddress verifies this as a IPv6 address and will return true. Hence resulting into incorrect formatting of complete string [2001:db8::1:8080].
Though your concern "This prevents incorrectly splitting IPv6 addresses that might have numeric suffixes that look like ports" is very much valid and I have handled these scenarios where I have made sure that IPv6 string is returned properly.
| } | ||
|
|
||
| // Remove any existing scheme if present | ||
| String cleanHostWithPort = hostWithPort.replaceFirst("^\\w+://", ""); |
There was a problem hiding this comment.
| String cleanHostWithPort = hostWithPort.replaceFirst("^\\w+://", ""); | |
| String cleanHostWithPort = hostWithPort.replaceFirst("^[a-zA-Z][a-zA-Z0-9+.-]*://", ""); |
is probably more accurate according to the RFC 3986
There was a problem hiding this comment.
Thank You, I have applied your suggestion.
| // Handle IPv6 address format using UrlUtils | ||
| if (host != null) { | ||
| host = UrlUtils.formatHostnameForUrl(host); | ||
| } |
There was a problem hiding this comment.
if host == null, is it ok to continue? it will probably throw some error in the builder
There was a problem hiding this comment.
Thank you for pointing that out, I have corrected the above
| output = newUri.toString(); | ||
| } | ||
| } catch (URISyntaxException e) { | ||
| // If there's an error parsing the URI, keeping the original URL |
There was a problem hiding this comment.
some debug log would be good here
There was a problem hiding this comment.
added, Thank You
| ).toString(); | ||
| } | ||
| } catch (URISyntaxException e) { | ||
| // Keep original if URI parsing fails |
There was a problem hiding this comment.
added, Thank You
| throw new IllegalArgumentException("Host cannot be null or empty"); | ||
| } | ||
|
|
||
| // Remove any existing scheme if present |
There was a problem hiding this comment.
is it possible that there will be scheme if the parameter is called hostWithPort?
There was a problem hiding this comment.
I have added this just to safeguard us if any caller passes value which may contain scheme, also as it's a single regex check hence, computationally cheap for us and makes sure correct host/host+port is sent.
...og-services/src/main/java/org/zowe/apiml/apicatalog/swagger/ApiDocRetrievalServiceLocal.java
Show resolved
Hide resolved
gateway-service/src/main/java/org/zowe/apiml/gateway/services/ServicesInfoService.java
Show resolved
Hide resolved
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
…host-formatting' into hrishikesh-nalawade/GH4318/IPv6-host-formatting
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
|
|
Signed-off-by: hrishikesh-nalawade <hrishikesh.nalawade.17@gmail.com>
2 participants
Description
Code changes to support IPv6 address
Linked to #4318
Type of change
Please delete options that are not relevant.
Checklist: