Issue #7277 - Updates based on review

Signed-off-by: Joakim Erdfelt <[email protected]>

jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java

@@ -164,7 +164,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co
     private ConnectionFactory _defaultConnectionFactory;
     /* The name used to link up virtual host configuration to named connectors */
     private String _name;
-    /* The authority used to override the connection local name/port (see ServletRequest.getLocalName(), and ServletRequest.getLocalPort()) */
+    /* The authority used to define the connection local name/port (see ServletRequest.getLocalName(), and ServletRequest.getLocalPort()). */
     private HostPort _localAuthority;
     private int _acceptorPriorityDelta = -2;
     private boolean _accepting = true;
@@ -302,40 +302,21 @@ public int getAcceptors()
         return _acceptors.length;
     }
 
-    /**
-     * @return Returns the optional local authority override
-     */
     @ManagedAttribute("local authority")
     public HostPort getLocalAuthority()
     {
         return _localAuthority;
     }
 
-    /**
-     * Optional override of connection local name/port used within application API layer
-     * when identifying the local host name/port of a connected endpoint.
-     *
-     * @param authority the full authority including host and port, or null to unset
-     */
-    public void setLocalAuthority(String authority)
-    {
-        setLocalAuthority(new HostPort(authority));
-    }
-
-    /**
-     * Optional override of connection local name/port used within application API layer
-     * when identifying the local host name/port of a connected endpoint.
-     *
-     * @param authority the full authority including host and port, or null to unset
-     */
     public void setLocalAuthority(HostPort authority)
     {
-        if (authority == null)
-            _localAuthority = null;
-        else if (!authority.hasHost() || !authority.hasPort())
-            throw new IllegalStateException("Local Authority must have host and port declared");
-        else
-            _localAuthority = authority;
+        if (isStarted())
+            throw new IllegalStateException(getState());
+
+        if (!authority.hasHost())
+            throw new IllegalStateException("Local Authority must have host declared");
+
+        _localAuthority = authority;
     }
 
     @Override
@@ -359,6 +340,10 @@ protected void doStart() throws Exception
 
         _lease = ThreadPoolBudget.leaseFrom(getExecutor(), this, _acceptors.length);
 
+        // default the local authority if unset by connector implementation or user
+        if (_localAuthority == null)
+            setLocalAuthority(new HostPort("localhost"));
+
         super.doStart();
 
         _stopping = new CountDownLatch(_acceptors.length);

jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNetworkConnector.java

@@ -23,6 +23,7 @@
 import java.util.concurrent.Future;
 
 import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.util.HostPort;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.thread.Scheduler;
@@ -35,7 +36,6 @@
 @ManagedObject("AbstractNetworkConnector")
 public abstract class AbstractNetworkConnector extends AbstractConnector implements NetworkConnector
 {
-
     private volatile String _host;
     private volatile int _port = 0;
 
@@ -74,10 +74,18 @@ public int getLocalPort()
         return -1;
     }
 
+    /**
+     * @return the local address host name, never null
+     */
+    protected abstract String getLocalName();
+
     @Override
     protected void doStart() throws Exception
     {
         open();
+        // define a default local authority if unspecified by user
+        if (getLocalAuthority() == null)
+            setLocalAuthority(new HostPort(getLocalName(), getLocalPort()));
         super.doStart();
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/Connector.java

@@ -25,6 +25,7 @@
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.util.HostPort;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.Container;
@@ -102,4 +103,17 @@ public interface Connector extends LifeCycle, Container, Graceful
      * @return The connector name or null.
      */
     String getName();
+
+    /**
+     * @return Returns the connection local authority (name/port).
+     */
+    HostPort getLocalAuthority();
+
+    /**
+     * Specify the connection local authority (name/port) used within application API layer
+     * when identifying the local host name/port of a connected endpoint.
+     *
+     * @param authority the full authority including host and port, or null to reset to default
+     */
+    void setLocalAuthority(HostPort authority);
 }

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java

@@ -97,7 +97,6 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
     private MetaData.Response _committedMetaData;
     private RequestLog _requestLog;
     private long _oldIdleTimeout;
-    private String overriddenLocalName;
 
     /**
      * Bytes written after interception (eg after compression)
@@ -308,7 +307,7 @@ public EndPoint getEndPoint()
      * if the local address is unavailable.
      * </p>
      * <p>
-     * Value can be overridden by {@link AbstractConnector#setLocalAuthority(HostPort)} for
+     * Value can be overridden by {@link Connector#setLocalAuthority(HostPort)} for
      * scenarios where Jetty is being an intermediary and the local name
      * needs to be changed to satisfy public (pre-intermediary) HTTP behaviors
      * such as absolute-URI creation (eg: Location response header).
@@ -319,14 +318,9 @@ public EndPoint getEndPoint()
      */
     public String getLocalName()
     {
-        Connector connector = getConnector();
-
-        if (connector instanceof AbstractConnector)
-        {
-            HostPort localAuthority = ((AbstractConnector)connector).getLocalAuthority();
-            if (localAuthority != null)
-                return localAuthority.getHost();
-        }
+        HostPort localAuthority = getConnector().getLocalAuthority();
+        if (localAuthority != null)
+            return localAuthority.getHost();
 
         InetSocketAddress local = getLocalAddress();
         if (local != null)
@@ -342,7 +336,7 @@ public String getLocalName()
      * This is the port the connector is bound to and is accepting connection on.
      * </p>
      * <p>
-     * Value can be overridden by {@link AbstractConnector#setLocalAuthority(HostPort)} for
+     * Value can be overridden by {@link Connector#setLocalAuthority(HostPort)} for
      * scenarios where Jetty is being an intermediary and the local port
      * needs to be changed to satisfy public (pre-intermediary) HTTP behaviors
      * such as absolute-URI creation (eg: Location response header).
@@ -353,14 +347,9 @@ public String getLocalName()
      */
     public int getLocalPort()
     {
-        Connector connector = getConnector();
-
-        if (connector instanceof AbstractConnector)
-        {
-            HostPort localAuthority = ((AbstractConnector)connector).getLocalAuthority();
-            if (localAuthority != null)
-                return localAuthority.getPort();
-        }
+        HostPort localAuthority = getConnector().getLocalAuthority();
+        if (localAuthority != null)
+            return localAuthority.getPort();
 
         InetSocketAddress local = getLocalAddress();
         return local == null ? 0 : local.getPort();

jetty-server/src/main/java/org/eclipse/jetty/server/LocalConnector.java

@@ -35,6 +35,7 @@
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.ByteArrayOutputStream2;
+import org.eclipse.jetty.util.HostPort;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.util.thread.Scheduler;
 
@@ -221,6 +222,14 @@ protected void accept(int acceptorID) throws IOException, InterruptedException
         connection.onOpen();
     }
 
+    @Override
+    protected void doStart() throws Exception
+    {
+        // Change the default authority name here so that it's obvious during testing
+        setLocalAuthority(new HostPort("connector.local."));
+        super.doStart();
+    }
+
     /**
      * Get a single response using a parser to search for the end of the message.
      *

jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -27,7 +27,6 @@
 import java.io.UnsupportedEncodingException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.net.UnknownHostException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.charset.UnsupportedCharsetException;
@@ -373,10 +372,10 @@ public void addEventListener(final EventListener listener)
         if (listener instanceof AsyncListener)
             throw new IllegalArgumentException(listener.getClass().toString());
     }
-    
+
     /**
      * Remember a session that this request has just entered.
-     * 
+     *
      * @param s the session
      */
     public void enterSession(HttpSession s)
@@ -404,9 +403,10 @@ private void leaveSession(Session session)
     }
 
     /**
-     * A response is being committed for a session, 
+     * A response is being committed for a session,
      * potentially write the session out before the
      * client receives the response.
+     *
      * @param session the session
      */
     private void commitSession(Session session)
@@ -991,30 +991,20 @@ public Enumeration<Locale> getLocales()
     @Override
     public String getLocalAddr()
     {
-        if (_channel == null)
+        if (_channel != null)
         {
-            try
-            {
-                String name = InetAddress.getLocalHost().getHostAddress();
-                if (StringUtil.ALL_INTERFACES.equals(name))
-                    return null;
-                return formatAddrOrHost(name);
-            }
-            catch (UnknownHostException e)
-            {
-                LOG.ignore(e);
-                return null;
-            }
+            InetSocketAddress local = _channel.getLocalAddress();
+            if (local == null)
+                return "";
+            InetAddress address = local.getAddress();
+            String result = address == null
+                ? local.getHostString()
+                : address.getHostAddress();
+
+            return formatAddrOrHost(result);
         }
 
-        InetSocketAddress local = _channel.getLocalAddress();
-        if (local == null)
-            return "";
-        InetAddress address = local.getAddress();
-        String result = address == null
-            ? local.getHostString()
-            : address.getHostAddress();
-        return formatAddrOrHost(result);
+        return "";
     }
 
     /*
@@ -1026,8 +1016,7 @@ public String getLocalName()
         if (_channel != null)
         {
             String localName = _channel.getLocalName();
-            if (localName != null)
-                return formatAddrOrHost(localName);
+            return formatAddrOrHost(localName);
         }
 
         return null;
@@ -1044,9 +1033,6 @@ public int getLocalPort()
             int localPort = _channel.getLocalPort();
             if (localPort > 0)
                 return localPort;
-            InetSocketAddress local = _channel.getLocalAddress();
-            if (local != null)
-                return local.getPort();
         }
         return 0;
     }
@@ -1428,15 +1414,6 @@ private String findServerName()
         if (name != null)
             return formatAddrOrHost(name);
 
-        // Return the local host
-        try
-        {
-            return formatAddrOrHost(InetAddress.getLocalHost().getHostAddress());
-        }
-        catch (UnknownHostException e)
-        {
-            LOG.ignore(e);
-        }
         return null;
     }
 
@@ -1453,9 +1430,10 @@ public int getServerPort()
         // If no port specified, return the default port for the scheme
         if (port <= 0)
         {
-            if (getScheme().equalsIgnoreCase(URIUtil.HTTPS))
+            if (HttpScheme.HTTPS.is(getScheme()))
                 return 443;
-            return 80;
+            else
+                return 80;
         }
 
         // return a specific port
@@ -1545,7 +1523,7 @@ public void onCompleted()
                 leaveSession(s);
         }
     }
-    
+
     /**
      * Called when a response is about to be committed, ie sent
      * back to the client
@@ -1561,7 +1539,7 @@ public void onResponseCommit()
 
     /**
      * Find a session that this request has already entered for the
-     * given SessionHandler 
+     * given SessionHandler
      *
      * @param sessionHandler the SessionHandler (ie context) to check
      * @return
@@ -1570,9 +1548,9 @@ public HttpSession getSession(SessionHandler sessionHandler)
     {
         if (_sessions == null || _sessions.size() == 0 || sessionHandler == null)
             return null;
-        
+
         HttpSession session = null;
-        
+
         for (HttpSession s:_sessions)
         {
             Session ss =  Session.class.cast(s);
@@ -1585,7 +1563,7 @@ public HttpSession getSession(SessionHandler sessionHandler)
         }
         return session;
     }
-    
+
     /*
      * @see javax.servlet.http.HttpServletRequest#getSession()
      */
@@ -1621,7 +1599,7 @@ public HttpSession getSession(boolean create)
         _session = _sessionHandler.newHttpSession(this);
         if (_session == null)
             throw new IllegalStateException("Create session failed");
-        
+
         HttpCookie cookie = _sessionHandler.getSessionCookie(_session, getContextPath(), isSecure());
         if (cookie != null)
             _channel.getResponse().replaceCookie(cookie);