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.

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

WebServer implementations should return -1 if not listening on a port #24606

Closed
wants to merge 1 commit into from

Conversation

@spartusch
Copy link
Contributor

@spartusch spartusch commented Dec 23, 2020

The interface org.springframework.boot.web.server.WebServer defines that getPort() returns -1 if the server isn't listening on a port. But the current implementations of the interface return 0 instead. This changes the implementations to return -1.

Please note: Jetty itself does return -2 as the local port after the connection has been closed. Therefore in its implementation getPort() maps all local ports <= 0 to -1 to match the method's contract.

See #24540

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 gh-24540
Copy link

@Koellewe Koellewe left a comment

Thorough work.

@philwebb philwebb modified the milestones: 2.4.2, 2.3.x Jan 4, 2021
@philwebb philwebb changed the title Let WebServer implementations return port -1 if not listening on a port WebServer implementations should return -1 if not listening on a port Jan 5, 2021
@philwebb philwebb self-assigned this Jan 5, 2021
philwebb added a commit that referenced this pull request Jan 6, 2021
Update `WebServer` implementations to return -1 from `getPort()` if
the server  isn't listening on a port. This aligns the implementations
with the interface Javadoc.

See gh-24606
philwebb added a commit that referenced this pull request Jan 6, 2021
@philwebb philwebb modified the milestones: 2.3.x, 2.3.8 Jan 6, 2021
@philwebb philwebb closed this in 7825921 Jan 6, 2021
@philwebb
Copy link
Member

@philwebb philwebb commented Jan 6, 2021

Thanks very much for the compressive PR @spartusch. This has been merged to 2.3.x, 2.4.x and master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants