Skip to content

Conversation

@roro1506HD
Copy link
Contributor

getClientAddress() and getAddress() implementations were inverted, this fixes it

@roro1506HD roro1506HD requested a review from a team as a code owner November 13, 2025 12:52
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Nov 13, 2025
@emilyy-dev emilyy-dev added the publish-pr Enables a workflow to build Paperclip jars on the pull request. label Nov 13, 2025
@papermc-pr-publishing
Copy link

Last updated for: f3f9fb9d3cb241647239a22c046d1a1b87fb278f.

Download the Paperclip jar for this pull request: paper-13307.zip

Maven Publication

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven("https://maven-prs.papermc.io/Paper/pr13307") {
        name = "Maven for PR #13307" // https://github.com/PaperMC/Paper/pull/13307
        mavenContent {
            includeModule("io.papermc.paper", "dev-bundle")
            includeModule("io.papermc.paper", "paper-api")
        }
    }
}

@Owen1212055
Copy link
Member

this.connection.channel.remoteAddress() can be null, can that be made nullable?

@roro1506HD
Copy link
Contributor Author

this.connection.channel.remoteAddress() can be null, can that be made nullable?

Can it really be null though? Like when the PaperPlayerLoginConnection object is created, the handshake has already been finished and the connection is already established and as per netty's javadoc, it should only return null if the connection is not yet established

But if you're sure it can be null, then the other method should be nullable as well since it's populated by channel.remoteAddress() when channelActive() is called in the Connection class

Copy link
Member

@emilyy-dev emilyy-dev left a comment

Choose a reason for hiding this comment

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

Whilst the method can return true according to the javadoc on various of the Channel subclasses, they all point out that is only the case before the channel can connect to a remote peer, but there is always a remote address due to the flow of operations, it is guaranteed to be non-null.
The same can be said about the handshake-resolved address, and before handshake it gets assigned to the channel address, neither of which points in time are available in the config API.
This looks good to me as it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

publish-pr Enables a workflow to build Paperclip jars on the pull request.

Projects

Status: Awaiting review

Development

Successfully merging this pull request may close these issues.

3 participants