[GitHub] massakam commented on a change in pull request #779: Ensure producer/consumer are creating before upgrading from HTTP to WebSocket

[gi...@git.apache.org]

massakam commented on a change in pull request #779: Ensure producer/consumer are creating before upgrading from HTTP to WebSocket
URL: https://github.com/apache/incubator-pulsar/pull/779#discussion_r140200366
 
 

 ##########
 File path: pulsar-websocket/src/main/java/org/apache/pulsar/websocket/AbstractWebSocketHandler.java
 ##########
 @@ -55,44 +56,56 @@ public AbstractWebSocketHandler(WebSocketService service, HttpServletRequest req
         request.getParameterMap().forEach((key, values) -> {
             queryParams.put(key, values[0]);
         });
-    }
 
-    @Override
-    public void onWebSocketConnect(Session session) {
-        super.onWebSocketConnect(session);
-        log.info("[{}] New WebSocket session on topic {}", session.getRemoteAddress(), topic);
+        checkAuth(response);
+    }
 
+    private void checkAuth(ServletUpgradeResponse response) {
         String authRole = "<none>";
         if (service.isAuthenticationEnabled()) {
             try {
                 authRole = service.getAuthenticationService().authenticateHttpRequest(request);
-                log.info("[{}] Authenticated WebSocket client {} on topic {}", session.getRemoteAddress(), authRole,
+                log.info("[{}] Authenticated WebSocket client {} on topic {}", request.getRemoteAddr(), authRole,
 
 Review comment:
   I think that `session.getRemoteAddress().toString()` returns IP and port (such as `/172.21.205.28:58954`) but `request.getRemoteAddr()` returns only IP (such as `172.21.205.28`).
   Is this difference acceptable?
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services