Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,18 @@ public interface Connection extends Closeable {
Certificate[] getPeerCertificates();

/**
* Keeps track if the other peer of this session presented a self-signed certificate. When
* using self-signed certificate for server-2-server sessions then SASL EXTERNAL will not be
* used and instead server-dialback will be preferred for verifying the identify of the remote
* server.
* Returns the remote port used by the connection.
*
* @return the remote port, or 0 when unavailable.
*/
default int getRemotePort() {
return 0;
}

/**
* Keeps track of whether the other peer of this session presented a self-signed certificate. When
* using a self-signed certificate for server-to-server sessions, SASL EXTERNAL will not be
* used and instead server dialback will be preferred for verifying the identity of the remote
*
* @param isSelfSigned true if the other peer presented a self-signed certificate.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,18 @@ public String getHostName() throws UnknownHostException {
}

/**
* Returns the port that the connection uses.
* Returns the remote port used by the connection.
*
* @return the port that the connection uses.
* @return the remote port, or 0 when unavailable.
* @deprecated This method is replaced by {@link #getRemotePort()}
*/
Comment thread
guusdk marked this conversation as resolved.
@Deprecated(forRemoval = true, since = "5.1.0") // Remove in or after Openfire 5.2.0.
public int getPort() {
return getRemotePort();
}

@Override
public int getRemotePort() {
return socket.getPort();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ public String getHostAddress() throws UnknownHostException {
return inetAddress.getHostAddress();
}

@Override
public int getRemotePort() {
final SocketAddress remoteAddress = channelHandlerContext.channel().remoteAddress();
if (remoteAddress != null && remoteAddress instanceof InetSocketAddress) {
return ((InetSocketAddress) remoteAddress).getPort();
}
return 0;
}

@Override
public String getHostName() throws UnknownHostException {
final SocketAddress remoteAddress = channelHandlerContext.channel().remoteAddress();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2007-2009 Jive Software, 2021-2023 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2007-2009 Jive Software, 2021-2026 Ignite Realtime Foundation. All rights reserved.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file doesn't seem to be modified. We don't need to change the copyright year when the file not changed.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2004-2009 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2004-2009 Jive Software, 2017-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -504,6 +504,15 @@ public String getHostAddress() throws UnknownHostException {
return connection.getHostAddress();
}

@Override
public int getRemotePort() {
Connection connection = conn;
if (connection == null) {
return 0;
}
return connection.getRemotePort();
}

@Override
public String getHostName() throws UnknownHostException {
Connection connection = conn;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2007-2009 Jive Software, 2021-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2007-2009 Jive Software, 2021-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -51,6 +51,7 @@ public abstract class RemoteSession implements Session {
private String serverName;
private String hostAddress;
private String hostName;
private int remotePort = -1;

public RemoteSession(byte[] nodeID, JID address) {
this.nodeID = nodeID;
Expand Down Expand Up @@ -180,6 +181,17 @@ public String getHostName() throws UnknownHostException {
return hostName;
}

@Override
public int getRemotePort() {
if (remotePort == -1) {
ClusterTask<Object> task = getRemoteSessionTask(RemoteSessionTask.Operation.getRemotePort);
Object result = doSynchronousClusterTask(task);
remotePort = result == null ? 0 : (Integer) result;
}
return remotePort;
Comment on lines +186 to +191
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don’t permanently cache an unavailable remote port.

At Line 189, null is converted to 0 and cached forever. If the lookup fails transiently (for example while detached), later calls won’t retry and can keep showing no port even when it becomes available.

Proposed minimal fix
 `@Override`
 public int getRemotePort() {
     if (remotePort == -1) {
         ClusterTask<Object> task = getRemoteSessionTask(RemoteSessionTask.Operation.getRemotePort);
         Object result = doSynchronousClusterTask(task);
-        remotePort = result == null ? 0 : (Integer) result;
+        if (result != null) {
+            remotePort = (Integer) result;
+        }
     }
-    return remotePort;
+    return remotePort == -1 ? 0 : remotePort;
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@xmppserver/src/main/java/org/jivesoftware/openfire/session/RemoteSession.java`
around lines 186 - 191, The code currently converts a null cluster lookup result
into 0 and caches it in the remotePort field, preventing future retries; change
the logic in RemoteSession (the remotePort initialization block that calls
getRemoteSessionTask and doSynchronousClusterTask) so that you only assign to
remotePort when result != null (e.g., remotePort = (Integer) result); if result
is null leave remotePort as the original sentinel (e.g., -1) so subsequent calls
will retry the cluster lookup.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MilanTyagi2004 I suggest that you apply the proposed minimal fix.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this still needs to be done?

}


public void deliverRawText(String text) {
doClusterTask(getDeliverRawTextTask(text));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2007-2009 Jive Software, 2021-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2007-2009 Jive Software, 2021-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -143,6 +143,13 @@ else if (operation == Operation.getHostName) {
Log.error("Error getting address of session: {}", getSession(), e);
}
}
else if (operation == Operation.getRemotePort) {
if (getSession().isDetached()) {
Log.debug("Unable to get remote port of detached session: {}", getSession());
} else {
result = getSession().getRemotePort();
}
}
else if (operation == Operation.validate) {
result = getSession().validate();
}
Expand Down Expand Up @@ -246,6 +253,7 @@ public enum Operation {
*/
getLocalDomain,
getAddress,
getValidatedDomains
getValidatedDomains,
getRemotePort
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2005-2008 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2005-2008 Jive Software, 2017-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -195,6 +195,15 @@ default boolean isAuthenticated() {
* @throws java.net.UnknownHostException if IP address of host could not be determined.
*/
String getHostAddress() throws UnknownHostException;

/**
* Returns the remote port used by the connection.
*
* @return the remote port, or 0 when unavailable.
*/
default int getRemotePort() {
return 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Throughout the code, there are two different values used to represent a 'unknown' remote port: 0 and -1. Unless it is important to have different values (I cannot immediately see a reason) I would use the same value everywhere.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this still needs to be done?

}
Comment thread
Fishbowler marked this conversation as resolved.

/**
* Gets the host name for this IP address.
Expand Down
20 changes: 19 additions & 1 deletion xmppserver/src/main/webapp/server-session-details.jsp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%@ page contentType="text/html; charset=UTF-8" %>
<%--
-
- Copyright (C) 2004-2008 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved.
- Copyright (C) 2004-2008 Jive Software, 2017-2026 Ignite Realtime Foundation. All rights reserved.
-
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -268,6 +268,7 @@
<th><fmt:message key="server.session.details.authentication"/></th>
<th><fmt:message key="server.session.details.tls_version"/></th>
<th><fmt:message key="server.session.details.cipher"/></th>
<th><fmt:message key="ports.port"/></th>
<th><fmt:message key="server.session.label.creation" /></th>
<th><fmt:message key="server.session.label.last_active" /></th>
<th><fmt:message key="server.session.details.incoming_statistics" /></th>
Expand Down Expand Up @@ -313,6 +314,14 @@
</c:choose>
<td><c:out value="${session.TLSProtocolName}"/></td>
<td><c:out value="${session.cipherSuiteName}"/></td>
<td>
<c:choose>
<c:when test="${session.remotePort gt 0}">
<c:out value="${session.remotePort}"/>
</c:when>
<c:otherwise>&nbsp;</c:otherwise>
</c:choose>
</td>
<td ><fmt:formatDate type="both" value="${session.creationDate}"/></td>
<td ><fmt:formatDate type="both" value="${session.lastActiveDate}"/></td>
<td style="text-align: center" ><fmt:formatNumber type="number" value="${session.numClientPackets}"/></td>
Expand All @@ -333,6 +342,7 @@
<th><fmt:message key="server.session.details.authentication"/></th>
<th><fmt:message key="server.session.details.tls_version"/></th>
<th><fmt:message key="server.session.details.cipher"/></th>
<th><fmt:message key="ports.port"/></th>
<th><fmt:message key="server.session.label.creation" /></th>
<th><fmt:message key="server.session.label.last_active" /></th>
<th><fmt:message key="server.session.details.incoming_statistics" /></th>
Expand Down Expand Up @@ -378,6 +388,14 @@
</c:choose>
<td><c:out value="${session.TLSProtocolName}"/></td>
<td><c:out value="${session.cipherSuiteName}"/></td>
<td>
<c:choose>
<c:when test="${session.remotePort gt 0}">
<c:out value="${session.remotePort}"/>
</c:when>
<c:otherwise>&nbsp;</c:otherwise>
</c:choose>
</td>
<td ><fmt:formatDate type="both" value="${session.creationDate}"/></td>
<td ><fmt:formatDate type="both" value="${session.lastActiveDate}"/></td>
<td style="text-align: center" ><fmt:formatNumber type="number" value="${session.numClientPackets}"/></td>
Expand Down
Loading