You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/01/29 13:57:51 UTC
svn commit: r1655638 - in /tomcat/trunk/java/org/apache: coyote/http11/
tomcat/util/net/
Author: markt
Date: Thu Jan 29 12:57:50 2015
New Revision: 1655638
URL: http://svn.apache.org/r1655638
Log:
Push obtaining the remote host and address down to the SocketWrapper
Modified:
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Thu Jan 29 12:57:50 2015
@@ -886,6 +886,22 @@ public abstract class AbstractHttp11Proc
setErrorState(ErrorState.CLOSE_NOW, null);
break;
}
+ case REQ_HOST_ADDR_ATTRIBUTE: {
+ if (socketWrapper == null) {
+ request.remoteAddr().recycle();
+ } else {
+ request.remoteAddr().setString(socketWrapper.getRemoteAddr());
+ }
+ break;
+ }
+ case REQ_HOST_ATTRIBUTE: {
+ if (socketWrapper == null) {
+ request.remoteHost().recycle();
+ } else {
+ request.remoteHost().setString(socketWrapper.getRemoteHost());
+ }
+ break;
+ }
default: {
actionInternal(actionCode, param);
break;
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Thu Jan 29 12:57:50 2015
@@ -109,22 +109,6 @@ public class Http11AprProcessor extends
long socketRef = socketWrapper.getSocket().longValue();
switch (actionCode) {
- case REQ_HOST_ADDR_ATTRIBUTE: {
- if (socketRef == 0) {
- request.remoteAddr().recycle();
- } else {
- if (socketWrapper.getRemoteAddr() == null) {
- try {
- long sa = Address.get(Socket.APR_REMOTE, socketRef);
- socketWrapper.setRemoteAddr(Address.getip(sa));
- } catch (Exception e) {
- log.warn(sm.getString("http11processor.socket.info"), e);
- }
- }
- request.remoteAddr().setString(socketWrapper.getRemoteAddr());
- }
- break;
- }
case REQ_LOCAL_NAME_ATTRIBUTE: {
if (socketRef == 0) {
request.localName().recycle();
@@ -141,31 +125,6 @@ public class Http11AprProcessor extends
}
break;
}
- case REQ_HOST_ATTRIBUTE: {
- if (socketRef == 0) {
- request.remoteHost().recycle();
- } else {
- if (socketWrapper.getRemoteHost() == null) {
- try {
- long sa = Address.get(Socket.APR_REMOTE, socketRef);
- socketWrapper.setRemoteHost(Address.getnameinfo(sa, 0));
- if (socketWrapper.getRemoteHost() == null) {
- if (socketWrapper.getRemoteAddr() == null) {
- socketWrapper.setRemoteAddr(Address.getip(sa));
- }
- if (socketWrapper.getRemoteAddr() != null) {
- socketWrapper.setRemoteHost(socketWrapper.getRemoteAddr());
- }
- }
- } catch (Exception e) {
- log.warn(sm.getString("http11processor.socket.info"), e);
- }
- } else {
- request.remoteHost().setString(socketWrapper.getRemoteHost());
- }
- }
- break;
- }
case REQ_LOCAL_ADDR_ATTRIBUTE: {
if (socketRef == 0) {
request.localAddr().recycle();
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java Thu Jan 29 12:57:50 2015
@@ -111,25 +111,6 @@ public class Http11Nio2Processor extends
public void actionInternal(ActionCode actionCode, Object param) {
switch (actionCode) {
- case REQ_HOST_ADDR_ATTRIBUTE: {
- if (socketWrapper == null || socketWrapper.getSocket() == null) {
- request.remoteAddr().recycle();
- } else {
- if (socketWrapper.getRemoteAddr() == null) {
- InetAddress inetAddr = null;
- try {
- inetAddr = ((InetSocketAddress) socketWrapper.getSocket().getIOChannel().getRemoteAddress()).getAddress();
- } catch (IOException e) {
- // Ignore
- }
- if (inetAddr != null) {
- socketWrapper.setRemoteAddr(inetAddr.getHostAddress());
- }
- }
- request.remoteAddr().setString(socketWrapper.getRemoteAddr());
- }
- break;
- }
case REQ_LOCAL_NAME_ATTRIBUTE: {
if (socketWrapper == null || socketWrapper.getSocket() == null) {
request.localName().recycle();
@@ -149,34 +130,6 @@ public class Http11Nio2Processor extends
}
break;
}
- case REQ_HOST_ATTRIBUTE: {
- if (socketWrapper == null || socketWrapper.getSocket() == null) {
- request.remoteHost().recycle();
- } else {
- if (socketWrapper.getRemoteHost() == null) {
- InetAddress inetAddr = null;
- try {
- inetAddr = ((InetSocketAddress) socketWrapper.getSocket().getIOChannel().getRemoteAddress()).getAddress();
- } catch (IOException e) {
- // Ignore
- }
- if (inetAddr != null) {
- socketWrapper.setRemoteHost(inetAddr.getHostName());
- }
- if (socketWrapper.getRemoteHost() == null) {
- if (socketWrapper.getRemoteAddr() == null &&
- inetAddr != null) {
- socketWrapper.setRemoteAddr(inetAddr.getHostAddress());
- }
- if (socketWrapper.getRemoteAddr() != null) {
- socketWrapper.setRemoteHost(socketWrapper.getRemoteAddr());
- }
- }
- }
- request.remoteHost().setString(socketWrapper.getRemoteHost());
- }
- break;
- }
case REQ_LOCAL_ADDR_ATTRIBUTE: {
if (socketWrapper == null || socketWrapper.getSocket() == null) {
request.localAddr().recycle();
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Thu Jan 29 12:57:50 2015
@@ -98,20 +98,6 @@ public class Http11NioProcessor extends
public void actionInternal(ActionCode actionCode, Object param) {
switch (actionCode) {
- case REQ_HOST_ADDR_ATTRIBUTE: {
- if (socketWrapper == null) {
- request.remoteAddr().recycle();
- } else {
- if (socketWrapper.getRemoteAddr() == null) {
- InetAddress inetAddr = socketWrapper.getSocket().getIOChannel().socket().getInetAddress();
- if (inetAddr != null) {
- socketWrapper.setRemoteAddr(inetAddr.getHostAddress());
- }
- }
- request.remoteAddr().setString(socketWrapper.getRemoteAddr());
- }
- break;
- }
case REQ_LOCAL_NAME_ATTRIBUTE: {
if (socketWrapper == null) {
request.localName().recycle();
@@ -126,29 +112,6 @@ public class Http11NioProcessor extends
}
break;
}
- case REQ_HOST_ATTRIBUTE: {
- if (socketWrapper == null) {
- request.remoteHost().recycle();
- } else {
- if (socketWrapper.getRemoteHost() == null) {
- InetAddress inetAddr = socketWrapper.getSocket().getIOChannel().socket().getInetAddress();
- if (inetAddr != null) {
- socketWrapper.setRemoteHost(inetAddr.getHostName());
- }
- if (socketWrapper.getRemoteHost() == null) {
- if (socketWrapper.getRemoteAddr() == null &&
- inetAddr != null) {
- socketWrapper.setRemoteAddr(inetAddr.getHostAddress());
- }
- if (socketWrapper.getRemoteAddr() != null) {
- socketWrapper.setRemoteHost(socketWrapper.getRemoteAddr());
- }
- }
- }
- request.remoteHost().setString(socketWrapper.getRemoteHost());
- }
- break;
- }
case REQ_LOCAL_ADDR_ATTRIBUTE: {
if (socketWrapper == null) {
request.localAddr().recycle();
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Thu Jan 29 12:57:50 2015
@@ -2652,5 +2652,38 @@ public class AprEndpoint extends Abstrac
((SendfileData) sendfileData).socket = getSocket().longValue();
return ((AprEndpoint) getEndpoint()).getSendfile().add((SendfileData) sendfileData);
}
+
+
+ @Override
+ protected void populateRemoteAddr() {
+ long socket = getSocket().longValue();
+ if (socket == 0) {
+ return;
+ }
+ try {
+ long sa = Address.get(Socket.APR_REMOTE, socket);
+ remoteAddr = Address.getip(sa);
+ } catch (Exception e) {
+ log.warn(sm.getString("endpoint.warn.noRemoteAddr", getSocket()), e);
+ }
+ }
+
+
+ @Override
+ protected void populateRemoteHost() {
+ long socket = getSocket().longValue();
+ if (socket == 0) {
+ return;
+ }
+ try {
+ long sa = Address.get(Socket.APR_REMOTE, socket);
+ remoteHost = Address.getnameinfo(sa, 0);
+ if (remoteAddr == null) {
+ remoteAddr = Address.getip(sa);
+ }
+ } catch (Exception e) {
+ log.warn(sm.getString("endpoint.warn.noRemoteHost", getSocket()), e);
+ }
+ }
}
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties Thu Jan 29 12:57:50 2015
@@ -24,6 +24,8 @@ endpoint.warn.noHonorCipherOrder='Honor
endpoint.warn.noInsecureReneg=Secure re-negotiation is not supported by the SSL library {0}
endpoint.warn.unlockAcceptorFailed=Acceptor thread [{0}] failed to unlock. Forcing hard socket shutdown.
endpoint.warn.executorShutdown=The executor associated with thread pool [{0}] has not fully shutdown. Some application threads may still be running.
+endpoint.warn.noRemoteAddr=Unable to determine remote address for socket [{0}]
+endpoint.warn.noRemoteHost=Unable to determine remote host for socket [{0}]
endpoint.debug.channelCloseFail=Failed to close channel
endpoint.debug.destroySocket=Destroying socket [{0}]
endpoint.debug.pollerAdd=Add to addList socket [{0}], timeout [{1}], flags [{2}]
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Jan 29 12:57:50 2015
@@ -1309,6 +1309,37 @@ public class Nio2Endpoint extends Abstra
setSendfileData((SendfileData) sendfileData);
return ((Nio2Endpoint) getEndpoint()).processSendfile(this);
}
+
+
+ @Override
+ protected void populateRemoteAddr() {
+ SocketAddress socketAddress = null;
+ try {
+ socketAddress = getSocket().getIOChannel().getRemoteAddress();
+ } catch (IOException e) {
+ // Ignore
+ }
+ if (socketAddress instanceof InetSocketAddress) {
+ remoteAddr = ((InetSocketAddress) socketAddress).getAddress().getHostAddress();
+ }
+ }
+
+
+ @Override
+ protected void populateRemoteHost() {
+ SocketAddress socketAddress = null;
+ try {
+ socketAddress = getSocket().getIOChannel().getRemoteAddress();
+ } catch (IOException e) {
+ log.warn(sm.getString("endpoint.warn.noRemoteHost", getSocket()), e);
+ }
+ if (socketAddress instanceof InetSocketAddress) {
+ remoteHost = ((InetSocketAddress) socketAddress).getAddress().getHostName();
+ if (remoteAddr == null) {
+ remoteAddr = ((InetSocketAddress) socketAddress).getAddress().getHostAddress();
+ }
+ }
+ }
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Jan 29 12:57:50 2015
@@ -21,6 +21,7 @@ import java.io.EOFException;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
+import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.Socket;
@@ -1535,6 +1536,7 @@ public class NioEndpoint extends Abstrac
return new SendfileData(filename, pos, length);
}
+
@Override
public SendfileState processSendfile(SendfileDataBase sendfileData) {
setSendfileData((SendfileData) sendfileData);
@@ -1543,6 +1545,27 @@ public class NioEndpoint extends Abstrac
// Might as well do the first write on this thread
return getSocket().getPoller().processSendfile(key, this, true);
}
+
+
+ @Override
+ protected void populateRemoteAddr() {
+ InetAddress inetAddr = getSocket().getIOChannel().socket().getInetAddress();
+ if (inetAddr != null) {
+ remoteAddr = inetAddr.getHostAddress();
+ }
+ }
+
+
+ @Override
+ protected void populateRemoteHost() {
+ InetAddress inetAddr = getSocket().getIOChannel().socket().getInetAddress();
+ if (inetAddr != null) {
+ remoteHost = inetAddr.getHostName();
+ if (remoteAddr == null) {
+ remoteAddr = inetAddr.getHostAddress();
+ }
+ }
+ }
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1655638&r1=1655637&r2=1655638&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Thu Jan 29 12:57:50 2015
@@ -46,12 +46,12 @@ public abstract class SocketWrapperBase<
/*
* Following cached for speed / reduced GC
*/
- private String localAddr = null;
- private String localName = null;
- private int localPort = -1;
- private String remoteAddr = null;
- private String remoteHost = null;
- private int remotePort = -1;
+ protected String localAddr = null;
+ protected String localName = null;
+ protected int localPort = -1;
+ protected String remoteAddr = null;
+ protected String remoteHost = null;
+ protected int remotePort = -1;
/*
* Used if block/non-blocking is set at the socket level. The client is
* responsible for the thread-safe use of this field via the locks provided.
@@ -166,10 +166,23 @@ public abstract class SocketWrapperBase<
public void setLocalAddr(String localAddr) {this.localAddr = localAddr; }
public int getRemotePort() { return remotePort; }
public void setRemotePort(int remotePort) {this.remotePort = remotePort; }
- public String getRemoteHost() { return remoteHost; }
- public void setRemoteHost(String remoteHost) {this.remoteHost = remoteHost; }
- public String getRemoteAddr() { return remoteAddr; }
- public void setRemoteAddr(String remoteAddr) {this.remoteAddr = remoteAddr; }
+
+ public String getRemoteHost() {
+ if (remoteHost == null) {
+ populateRemoteHost();
+ }
+ return remoteHost;
+ }
+ protected abstract void populateRemoteHost();
+
+ public String getRemoteAddr() {
+ if (remoteAddr == null) {
+ populateRemoteAddr();
+ }
+ return remoteAddr;
+ }
+ protected abstract void populateRemoteAddr();
+
public boolean getBlockingStatus() { return blockingStatus; }
public void setBlockingStatus(boolean blockingStatus) {
this.blockingStatus = blockingStatus;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1655638 - in /tomcat/trunk/java/org/apache: coyote/http11/
tomcat/util/net/
Posted by Mark Thomas <ma...@apache.org>.
On 29/01/2015 13:50, Rémy Maucherat wrote:
> 2015-01-29 13:57 GMT+01:00 <ma...@apache.org>:
>
>> Author: markt
>> Date: Thu Jan 29 12:57:50 2015
>> New Revision: 1655638
>>
>> URL: http://svn.apache.org/r1655638
>> Log:
>> Push obtaining the remote host and address down to the SocketWrapper
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
>> tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
>> tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
>>
> This doesn't actually make code smaller (or simpler), so the idea is to
> remove these classes to have all IO API specific items in tomcat.util.net ?
Broadly, yes. This is part of what needs to be done to get to a single
Http11Processor class rather than than the abstract base class and 3
concrete implementations we have now. I was waiting for some unit tests
to complete so I did part of it and then committed it. The rest will
follow as time permits.
The end goal is to have all of the I/O implementation specific code in
Endpoint and SocketWrapper. Endpoint holds the 'per connector' state and
SocketWrapper the 'per connection' state.
While I am doing some clean-up in Endpoint and SocketWrapper as the
opportunity presents itself, I haven't gone over them in detail. I'm
sure there is some stuff I have pushed down to Endpoint/SocketWrapper
that could be cleaner but I plan to address that later.
I'm confident I'll be able to get to a single Http11Processor class. I'm
not so sure about the [Ajp|Http11]Protocol classes. There are two parts
to that. The I/O specific code in the ConnectionHandlers and the I/O
specific configuration options. It might end up being cleaner leaving
those broadly as they are.
The rough plan I have in my head at the moment is:
- Get to a single Http11Processor
- Take another look at [Ajp|Http11]Protocol and see if anything else
can be pushed down easily
- Refactor WebSocket to use the I/O layer directly rather than going
via the Servlet API
- Review Endpoint & SocketWrapper for clean-up opportunities
- Start thinking about HTTP/2.0
Note that some of the changes are am making now are with an eye to the
refactoring I plan to do in later stages. I'm sure some of those changes
will prove to be not quite what is required and further tweaking will be
required.
Overall, my aim is to end up with something that is easier to
understand, easier to maintain and easier to extend for future features
like HTTP/2.0. Personally, I have found the refactored code much easier
to work with. As I have made mistakes in the refactoring and triggered
unit test failures, debugging those mistakes has simpler and quicker
than I have come to expect for I/O layer bugs.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1655638 - in /tomcat/trunk/java/org/apache:
coyote/http11/ tomcat/util/net/
Posted by Rémy Maucherat <re...@apache.org>.
2015-01-29 13:57 GMT+01:00 <ma...@apache.org>:
> Author: markt
> Date: Thu Jan 29 12:57:50 2015
> New Revision: 1655638
>
> URL: http://svn.apache.org/r1655638
> Log:
> Push obtaining the remote host and address down to the SocketWrapper
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
>
> This doesn't actually make code smaller (or simpler), so the idea is to
remove these classes to have all IO API specific items in tomcat.util.net ?
Rémy