Skip to content

ApacheDSContainer should allow a zero port #8144

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

Closed
jzheaux opened this issue Mar 18, 2020 · 5 comments
Closed

ApacheDSContainer should allow a zero port #8144

jzheaux opened this issue Mar 18, 2020 · 5 comments
Assignees
Labels
in: ldap An issue in spring-security-ldap status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Mar 18, 2020

ApacheDS's TcpTransport allows for a zero port, selecting any available port at startup. It does not correctly propagate that port up through the object graph, though.

For example, giving a TcpTransport a port of zero:

server = new LdapServer();
TcpTransport transport = new TcpTransport(port);
server.setTransports(transport);
server.start();

will result in a server that is listening on a random port, but server.getPort() still returns 0.

Because ApacheDS has not had a GA release in many years, it's unlikely that this enhancement will get applied to the ApacheDS project.

Still, it would be nice if ApacheDSContainer could accept 0 as a port value. One immediate benefit from this is reducing the likelihood of a port collision when multiple LDAP servers are started up simultaneously.

For this to happen, the following test would need to pass:

ApacheDSContainer container = new ApacheDSContainer("dc=springframework,dc=org",
		"classpath:test-server.ldif");
container.setPort(0);
container.afterPropertiesSet();
assertNotEquals(container.getPort(), 0);
assertNotEquals(container.server.getPort(), 0);
assertNotEquals(container.server.getPortSSL(), 0);

It may be possible to post-process the server instance in ApacheDSContainer#afterPropertiesSet into a state where the selected port is correctly returned.

Note that UnboundIdContainerTests has a test that would likely be good to port over into ApacheDSContainerTests for testing this feature.

@jzheaux jzheaux added in: ldap An issue in spring-security-ldap type: enhancement A general enhancement status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Mar 18, 2020
@rwinch
Copy link
Member

rwinch commented Mar 18, 2020

Perhaps we can add another accessor on 'ApacheDsContsiner' that would be be used for getting the local port vs configured port?

@jzheaux
Copy link
Contributor Author

jzheaux commented Mar 25, 2020

That could be nice, yes. I think it should be getConfiguredPort so that the same pattern can be applied to UnboundIdContainer which already supports a zero port.

@evgeniycheban
Copy link
Contributor

I can take this task.

@jzheaux
Copy link
Contributor Author

jzheaux commented Apr 15, 2020

Thanks, @evgeniycheban! It's yours.

@jzheaux jzheaux self-assigned this Apr 15, 2020
@jzheaux jzheaux removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Apr 15, 2020
@jzheaux jzheaux assigned rwinch and unassigned jzheaux Apr 17, 2020
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue May 19, 2020
@rwinch rwinch closed this as completed in 0fa339f May 21, 2020
@rwinch rwinch added the status: duplicate A duplicate of another issue label May 21, 2020
@rwinch
Copy link
Member

rwinch commented May 21, 2020

Marking as duplicate of the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: ldap An issue in spring-security-ldap status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants