exceptionfactory commented on code in PR #10154:
URL: https://github.com/apache/nifi/pull/10154#discussion_r2246753365
##########
nifi-extension-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketServer.java:
##########
@@ -254,6 +261,21 @@ public Object createWebSocket(JettyServerUpgradeRequest
servletUpgradeRequest, J
}
}
+ private static int getPort(JettyServerUpgradeRequest
servletUpgradeRequest) {
+ final Object localSocketAddress =
servletUpgradeRequest.getLocalSocketAddress();
+ if (localSocketAddress == null) {
+ throw new RuntimeException("Local socket address is null");
+ }
+
+ // Check if the local socket address is an instance of
InetSocketAddress
+ if (!(localSocketAddress instanceof InetSocketAddress)) {
+ throw new RuntimeException("Local socket address is not an
instance of InetSocketAddress: " + localSocketAddress.getClass().getName());
+ }
Review Comment:
This check could also cover the `null` case, as long as the exception does
not attempt to get the class name and instead just passes the
`localSocketAddress`
##########
nifi-extension-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketServer.java:
##########
@@ -254,6 +261,21 @@ public Object createWebSocket(JettyServerUpgradeRequest
servletUpgradeRequest, J
}
}
+ private static int getPort(JettyServerUpgradeRequest
servletUpgradeRequest) {
+ final Object localSocketAddress =
servletUpgradeRequest.getLocalSocketAddress();
+ if (localSocketAddress == null) {
+ throw new RuntimeException("Local socket address is null");
Review Comment:
This seems better reported as an `IllegalStateException`:
```suggestion
throw new IllegalStateException("Local socket address is null");
```
##########
nifi-extension-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketServer.java:
##########
@@ -254,6 +261,21 @@ public Object createWebSocket(JettyServerUpgradeRequest
servletUpgradeRequest, J
}
}
+ private static int getPort(JettyServerUpgradeRequest
servletUpgradeRequest) {
+ final Object localSocketAddress =
servletUpgradeRequest.getLocalSocketAddress();
+ if (localSocketAddress == null) {
+ throw new RuntimeException("Local socket address is null");
+ }
+
+ // Check if the local socket address is an instance of
InetSocketAddress
+ if (!(localSocketAddress instanceof InetSocketAddress)) {
+ throw new RuntimeException("Local socket address is not an
instance of InetSocketAddress: " + localSocketAddress.getClass().getName());
+ }
+
+ final int port = ((InetSocketAddress) localSocketAddress).getPort();
+ return port;
Review Comment:
This could be consolidated with the above check using `instance of
InetSocketAddres socketAddress` and then using the named `socketAddress`
variable.
##########
nifi-extension-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/RoutingWebSocketListener.java:
##########
@@ -27,6 +27,7 @@
public class RoutingWebSocketListener extends AbstractAutoDemanding {
private final WebSocketMessageRouter router;
private String sessionId;
+ private Boolean secure;
Review Comment:
Is this nullable? The set method only takes a primitive `boolean`.
##########
nifi-extension-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketSession.java:
##########
@@ -66,7 +72,15 @@ public InetSocketAddress getLocalAddress() {
@Override
public boolean isSecure() {
- return session.isSecure();
+ return secure;
+ }
+
+ private static boolean checkSecure(final Session session) {
Review Comment:
Recommend adjust the method name.
```suggestion
private static boolean isSessionSecure(final Session session) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
