Skip to content

Fixed Unix Domain Socket Patch#9086

Closed
0xIO32 wants to merge 4 commits into
PaperMC:masterfrom
0xIO32:fix/uds
Closed

Fixed Unix Domain Socket Patch#9086
0xIO32 wants to merge 4 commits into
PaperMC:masterfrom
0xIO32:fix/uds

Conversation

@0xIO32

@0xIO32 0xIO32 commented Apr 2, 2023

Copy link
Copy Markdown

It Fixes that an exception is thrown when using a Unix Domain Socket.

There was already a Pull Request for this, but i made a new one, because it has added a new patch instead of updating an existing: #9084

@Owen1212055 Owen1212055 left a comment

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.

Please wrap your diff in paper comments, and use fully qualified names to avoid imports.

@Warriorrrr

Copy link
Copy Markdown
Member

@MarkusTieger Any chance you could revisit this PR and address Owen's comment? If you don't have time to update it we could update/supersede this ourselves.

@0xIO32

0xIO32 commented Sep 8, 2023

Copy link
Copy Markdown
Author

@MarkusTieger Any chance you could revisit this PR and address Owen's comment? If you don't have time to update it we could update/supersede this ourselves.

Sorry, i completely forgot about this pr. I will update it tomorrow and address the comments...

@0xIO32

0xIO32 commented Sep 9, 2023

Copy link
Copy Markdown
Author

If you need something to test: (Velocity 3 does not support it)
https://github.com/MarkusTieger/Velocity/tree/dev/3.0.0
(from an older velocity pr, that wasn't merge because it broke the api. I updated it and it still works fine. Velocity 5.0.0 does support uds, but not 1.20.1)

@0xIO32 0xIO32 requested a review from Owen1212055 September 9, 2023 17:08

@Warriorrrr Warriorrrr left a comment

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.

Still missing some paper comments, will give this a test later today

Comment thread patches/server/0608-Add-Unix-domain-socket-support.patch Outdated
Comment thread patches/server/0608-Add-Unix-domain-socket-support.patch Outdated
+ // Paper start
+ java.net.SocketAddress rawSocketAddress = connection.channel.remoteAddress();
+ java.net.InetAddress rawAddress = java.net.InetAddress.getLoopbackAddress();
+ if (rawSocketAddress != null && rawSocketAddress instanceof java.net.InetSocketAddress rawInetSocketAddress) {

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.

Null check here can be removed here and in the other spot, the instanceof takes care of it

electronicboy
electronicboy previously approved these changes Sep 16, 2023
@lynxplay lynxplay dismissed electronicboy’s stale review September 16, 2023 23:54

Wrong, this is stupid

@lynxplay

Copy link
Copy Markdown
Contributor

After some further discussion, this should properly address the API as well.
imo best to pass the entire SocketAddress to the PlayerLoginEvent and AsyncPlayerPreLoginEvent, deprecated the existing
methods yielded InetAddress and having them do the fallback, e.g. the API would have the

if (this.socketAddr instanceOf InetSocketAddress inet) return inet.getAddress();
return getLoopback()

and adding a new method that yields the plain socket addr.
Additionally, mark the constructors you change with @ApiStatus.Internal

@0xIO32

0xIO32 commented Sep 26, 2023

Copy link
Copy Markdown
Author

Would be great, if someone could rebase it, because there are conflicts and i never actually dealt with them in patches.
(Let's say i tryed, and after that, the applyPatches task failed)

@0xIO32

0xIO32 commented Nov 11, 2024

Copy link
Copy Markdown
Author

tbh I don't want to deal with this pr anymore.

@0xIO32 0xIO32 closed this Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

5 participants