Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WebServer.getPort() returns 0 instead of -1 for closed connections #24540

Closed
philwebb opened this issue Dec 16, 2020 · 7 comments
Closed

WebServer.getPort() returns 0 instead of -1 for closed connections #24540

philwebb opened this issue Dec 16, 2020 · 7 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@philwebb
Copy link
Member

The javadoc for WebServer.getPort() states that it returns "the port (or -1 if none)". The implementations appear to return 0.
@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 16, 2020
@philwebb
Copy link
Member Author

Flagging to see if we should change the code or the javadoc.

@mbhave
Copy link
Contributor

mbhave commented Dec 17, 2020

When setting the port 0 is used for selecting a random port and -1 to disable. That makes me think the javadoc is correct.

@wilkinsona
Copy link
Member

The implementations could be correct too as I think we need to distinguish between being disabled (-1) and not knowing what the port is because the connector's stopped and it's configured to use an ephemeral port (0).

@mbhave
Copy link
Contributor

mbhave commented Dec 17, 2020

Good point @wilkinsona.

@philwebb
Copy link
Member Author

We're going to change the implementation not the javadoc

@philwebb philwebb added this to the 2.3.x milestone Dec 18, 2020
@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 18, 2020
spartusch added a commit to spartusch/spring-boot that referenced this issue Dec 23, 2020
@spartusch
Let WebServer implementations return port -1 if not listening on a port
384eab6 
The interface org.springframework.boot.web.server.WebServer defines
that getPort() returns -1 if the server isn't listening on a port. This
commit changes the implementations of the interface accordingly.

See spring-projectsgh-24540
@spartusch spartusch mentioned this issue Dec 23, 2020
Closed
@spartusch
Copy link
Contributor

Hi everyone, I've created a PR for this issue #24606. I hope that's ok. It's my first contribution so I'm not familiar with the processes. In general the PR changes the implementations to return -1 instead of 0.

@philwebb
Copy link
Member Author

philwebb commented Jan 4, 2021 •
edited

Thanks @spartusch, I'll close this issue in favor of PR #24606 and we can continue any discussion there when we get a chance to review it.

@philwebb philwebb closed this as completed Jan 4, 2021
@philwebb philwebb removed this from the 2.3.x milestone Jan 4, 2021
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed type: bug A general bug labels Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@philwebb @wilkinsona @spartusch @mbhave