Fix setting of public port different from YaCy port - #792
Open
mmeier86 wants to merge 4 commits into
Open
Conversation
The old "publicPort" setting didn't seem to have any effect at all, peers still tried to access the instance on the "port" value. This commit changes the getPublicPort function to decide on which port to return without receiving an explicit key. This centralizes the decision on which port is communicated to other peers.
If publicPort is set at startup and port.public is not, the publicPort value is written into port.public.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a fix for #791.
I've had a look at the code, and it looks like the
getPublicPortfunction was already used pretty consistently to get the public port send to peers, and didn't seem to be used for anything else. But it always expected the config key of the port setting to be handed in as an argument.Instead of continuing that, I rewrote the function to instead only receive whether SSL is in use or not, and then decide where to fetch the port internally, doing the following:
SERVER_PORT_PUBLICis set, return that valueSERVER_SSLPORTfrom config or from UPnP mapSERVER_PORTfrom config or from UPnP mapI've decided to introduce the new
SERVER_PORT_PUBLIC/port.publicsetting instead of re-using thepublicPortsetting because for some reason, settingpublicPortvia the env variableYACY_PUBLICPORTdid not work, and I'm still not sure why.To make migration easier, I also introduced a migration step: If
publicPortis set andport.publicis not,port.publicis automatically set to the value ofpublicPort.One last point is my change in the
SettingsAck_p.javafile. I changed the code to callupdateMySeed, because at least for me, the old way with just setting the seed's port didn't do anything, at least in all the setups I've tried, while callingupdateMySeedupdated the port send to peers for connections as expected.Once I've got the okay about the introduction of the new
port.public, I would also be happy to create another PR for the website to update the docs.