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

Remove DNS lookups during websocket connection initiation #28280

Closed
ud1 opened this issue Apr 4, 2022 · 3 comments
Closed

Remove DNS lookups during websocket connection initiation #28280

ud1 opened this issue Apr 4, 2022 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@ud1
Copy link

ud1 commented Apr 4, 2022

The ServletServerHttpRequest getLocalAddress and getRemoteAddress methods contain additional DNS queries. The servletRequest.getLocalName() method requests a hostname from an IP address, and the InetSocketAddress constructor calls InetAddress.getByName, which requests an IP address from a hostname.

It looks like these requests can be omitted if you use servletRequest.getLocalAddr() and servletRequest.getRemoteAddr() instead of get*Name() methods.

If DNS is not properly configured on the server, then the establishment of a websocket connection hangs (for a few seconds).

Problematic stack trace:

	at java.net.Inet4AddressImpl.getHostByAddr(Native Method)
	at java.net.InetAddress$2.getHostByAddr(InetAddress.java:933)
	at java.net.InetAddress.getHostFromNameService(InetAddress.java:618)
	at java.net.InetAddress.getHostName(InetAddress.java:560)
	at java.net.InetAddress.getHostName(InetAddress.java:532)
	at org.apache.tomcat.util.net.NioEndpoint$NioSocketWrapper.populateLocalName(NioEndpoint.java:1506)
	at org.apache.tomcat.util.net.SocketWrapperBase.getLocalName(SocketWrapperBase.java:277)
	at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:478)
	at org.apache.coyote.Request.action(Request.java:517)
	at org.apache.catalina.connector.Request.getLocalName(Request.java:1357)
	at org.apache.catalina.connector.RequestFacade.getLocalName(RequestFacade.java:1002)
	at org.springframework.http.server.ServletServerHttpRequest.getLocalAddress(ServletServerHttpRequest.java:200)
	at org.springframework.web.socket.server.standard.AbstractStandardUpgradeStrategy.upgrade(AbstractStandardUpgradeStrategy.java:116)
	at org.springframework.web.socket.server.support.AbstractHandshakeHandler.doHandshake(AbstractHandshakeHandler.java:297)
	at org.springframework.web.socket.server.support.WebSocketHttpRequestHandler.handleRequest(WebSocketHttpRequestHandler.java:178)
	at org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter.handle(HttpRequestHandlerAdapter.java:52)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1067)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)

The impression is that these addresses are not even used by Spring, but are only needed to ensure the operation of the WebSocketSession methods getLocalAddress() and getRemoteAddress(), and we don't use these methods at all.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 4, 2022
@ud1
Copy link
Author

ud1 commented Apr 4, 2022

There are two classes named ServletServerHttpRequest, first one org.springframework.http.server.ServletServerHttpRequest and the second org.springframework.http.server.reactive.ServletServerHttpRequest.
The reactive.ServletServerHttpRequest getLocalAddress method already uses getLocalAddr() method.

@jhoeller jhoeller self-assigned this Apr 4, 2022
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 4, 2022
@jhoeller jhoeller added this to the 5.3.19 milestone Apr 4, 2022
@jhoeller jhoeller added the status: backported An issue that has been backported to maintenance branches label Apr 4, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
RangerRick pushed a commit to opennms-forge/spring-framework that referenced this issue Jul 26, 2022
@lrgrz
Copy link

lrgrz commented Sep 19, 2022

I'm afraid the issue is not resolved.

The AbstractStandardUpgradeStrategy still tries to get InetSocketAddresses during upgrade:

	@Override
	public void upgrade(ServerHttpRequest request, ServerHttpResponse response,
			@Nullable String selectedProtocol, List<WebSocketExtension> selectedExtensions,
			@Nullable Principal user, WebSocketHandler wsHandler, Map<String, Object> attrs)
			throws HandshakeFailureException {

		HttpHeaders headers = request.getHeaders();
		InetSocketAddress localAddr = null;
		try {
			localAddr = request.getLocalAddress();
		}
		catch (Exception ex) {
			// Ignore
		}
		InetSocketAddress remoteAddr = null;
		try {
			remoteAddr = request.getRemoteAddress();
		}
		catch (Exception ex) {
			// Ignore
		}

and ServerHttpRequest uses constructor of InetSocketAddress, which performs DNS lookup:

	@Override
	public InetSocketAddress getLocalAddress() {
		return new InetSocketAddress(this.servletRequest.getLocalAddr(), this.servletRequest.getLocalPort());
	}

	@Override
	public InetSocketAddress getRemoteAddress() {
		return new InetSocketAddress(this.servletRequest.getRemoteHost(), this.servletRequest.getRemotePort());
	}

(checked on spring 5.3.20 and java 17)

@sbrannen
Copy link
Member

@lrgrz, this issue was closed back in April.

If you think you have discovered a bug or regression, please create a new issue to address that.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants