Resolved errorprone ReferenceEquality warnings #2206

Objects which inherit or implement an `equals()` method should not be compared with == or !=
When the comparison of references is intentional `@SuppressWarnings("ReferenceEquality")` can be used

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java

@@ -417,7 +417,9 @@ public class GZIPContentDecoder implements Destroyable
      */
     public void release(ByteBuffer buffer)
     {
-        if (_pool!=null && buffer!=BufferUtil.EMPTY_BUFFER)
+        @SuppressWarnings("ReferenceEquality")
+        boolean isBufferEmpty = (buffer!=BufferUtil.EMPTY_BUFFER);
+        if (_pool!=null && isBufferEmpty)
             _pool.release(buffer);
     }
 }

jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java

@@ -276,9 +276,12 @@ public class HttpField
 
     public boolean isSameName(HttpField field)
     {
+        @SuppressWarnings("ReferenceEquality")
+        boolean sameObject = (field==this);
+
         if (field==null)
             return false;
-        if (field==this)
+        if (sameObject)
             return true;
         if (_header!=null && _header==field.getHeader())
             return true;

jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java

@@ -202,7 +202,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint
                 throw new ClosedChannelException();
 
             ByteBuffer in = _inQ.peek();
-            if (BufferUtil.hasContent(in) || in==EOF)
+            if (BufferUtil.hasContent(in) || isEOF(in))
                 execute(_runFillable);
         }
     }
@@ -224,7 +224,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint
         boolean fillable=false;
         try(Locker.Lock lock = _locker.lock())
         {
-            if (_inQ.peek()==EOF)
+            if (isEOF(_inQ.peek()))
                 throw new RuntimeIOException(new EOFException());
             boolean was_empty=_inQ.isEmpty();
             if (in==null)
@@ -248,7 +248,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint
         boolean fillable=false;
         try(Locker.Lock lock = _locker.lock())
         {
-            if (_inQ.peek()==EOF)
+            if (isEOF(_inQ.peek()))
                 throw new RuntimeIOException(new EOFException());
             boolean was_empty=_inQ.isEmpty();
             if (in==null)
@@ -415,7 +415,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint
                     break;
 
                 ByteBuffer in= _inQ.peek();
-                if (in==EOF)
+                if (isEOF(in))
                 {
                     filled=-1;
                     break;
@@ -549,4 +549,16 @@ public class ByteArrayEndPoint extends AbstractEndPoint
         return String.format("%s[q=%d,q[0]=%s,o=%s]",super.toString(),q,b,o);
     }
 
+    /* ------------------------------------------------------------ */
+    /**
+     * Compares a ByteBuffer Object to EOF by Reference
+     * @param buffer the input ByteBuffer to be compared to EOF
+     * @return Whether the reference buffer is equal to that of EOF
+     */
+    private static boolean isEOF(ByteBuffer buffer)
+    {
+        @SuppressWarnings("ReferenceEquality")
+        boolean is_EOF = (buffer==EOF);
+        return is_EOF;
+    }
 }

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java

@@ -626,8 +626,12 @@ public class SslConnection extends AbstractConnection
 
                         // We also need an app buffer, but can use the passed buffer if it is big enough
                         ByteBuffer app_in;
+                        boolean used_passed_buffer = false;
                         if (BufferUtil.space(buffer) > _sslEngine.getSession().getApplicationBufferSize())
+                        {
                             app_in = buffer;
+                            used_passed_buffer = true;
+                        }
                         else if (_decryptedInput == null)
                             app_in = _decryptedInput = _bufferPool.acquire(_sslEngine.getSession().getApplicationBufferSize(), _decryptedDirectBuffers);
                         else
@@ -729,7 +733,7 @@ public class SslConnection extends AbstractConnection
                                         // another call to fill() or flush().
                                         if (unwrapResult.bytesProduced() > 0)
                                         {
-                                            if (app_in == buffer)
+                                            if (used_passed_buffer)
                                                 return unwrapResult.bytesProduced();
                                             return BufferUtil.append(buffer,_decryptedInput);
                                         }

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

@@ -427,7 +427,10 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
         if (LOG.isDebugEnabled())
             LOG.debug("upgrade {} {}", this, _upgrade);
 
-        if (_upgrade != PREAMBLE_UPGRADE_H2C && (_connection == null || !_connection.contains("upgrade")))
+        @SuppressWarnings("ReferenceEquality")
+        boolean isUpgraded_H2C = (_upgrade == PREAMBLE_UPGRADE_H2C);
+
+        if (!isUpgraded_H2C && (_connection == null || !_connection.contains("upgrade")))
             throw new BadMessageException(HttpStatus.BAD_REQUEST_400);
 
         // Find the upgrade factory
@@ -464,7 +467,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
         // Send 101 if needed
         try
         {
-            if (_upgrade != PREAMBLE_UPGRADE_H2C)
+            if (!isUpgraded_H2C)
                 sendResponse(new MetaData.Response(HttpVersion.HTTP_1_1, HttpStatus.SWITCHING_PROTOCOLS_101, response101, 0), null, true);
         }
         catch (IOException e)

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

@@ -143,6 +143,18 @@ public class Request implements HttpServletRequest
 
     private static final MultiMap<String> NO_PARAMS = new MultiMap<>();
 
+    /* ------------------------------------------------------------ */
+    /**
+     * Compare inputParameters to NO_PARAMS by Reference
+     * @param inputParameters The parameters to compare to NO_PARAMS
+     * @return True if the inputParameters reference is equal to NO_PARAMS otherwise False
+     */
+    private static boolean isNoParams(MultiMap<String> inputParameters) {
+        @SuppressWarnings("ReferenceEquality")
+        boolean is_no_params = (inputParameters==NO_PARAMS);
+        return is_no_params;
+    }
+
     /* ------------------------------------------------------------ */
     /**
      * Obtain the base {@link Request} instance of a {@link ServletRequest}, by
@@ -397,9 +409,9 @@ public class Request implements HttpServletRequest
         }
 
         // Do parameters need to be combined?
-        if (_queryParameters==NO_PARAMS || _queryParameters.size()==0)
+        if (isNoParams(_queryParameters) || _queryParameters.size()==0)
             _parameters=_contentParameters;
-        else if (_contentParameters==NO_PARAMS || _contentParameters.size()==0)
+        else if (isNoParams(_contentParameters) || _contentParameters.size()==0)
             _parameters=_queryParameters;
         else
         {

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

@@ -456,11 +456,29 @@ public class ResponseWriter extends PrintWriter
     { 
         try 
         {
+            
+            if(l==null) 
+                l = _locale;
+            
             synchronized (lock) 
             {
                 isOpen();
-                if ((_formatter == null) || (_formatter.locale() != l))
+
+                if(_formatter == null)
+                {
                     _formatter = new Formatter(this, l);
+                } 
+                else if(_formatter.locale() != null) 
+                {
+                    if(!_formatter.locale().equals(l))
+                        _formatter = new Formatter(this, l);
+                } 
+                else 
+                {
+                    if(l != null)
+                        _formatter = new Formatter(this, l);
+                }
+                
                 _formatter.format(l, format, args);
             }
         } 

jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java

@@ -63,6 +63,7 @@ import org.eclipse.jetty.server.session.Session;
 import org.eclipse.jetty.server.session.SessionData;
 import org.eclipse.jetty.server.session.SessionHandler;
 import org.eclipse.jetty.toolchain.test.AdvancedRunner;
+import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.thread.Scheduler;
 import org.eclipse.jetty.util.thread.TimerScheduler;
@@ -112,10 +113,13 @@ public class ResponseTest
     
     private Server _server;
     private HttpChannel _channel;
+    private ByteBuffer _content = BufferUtil.allocate(16*1024);
 
     @Before
     public void init() throws Exception
     {
+        BufferUtil.clear(_content);
+        
         _server = new Server();
         Scheduler _scheduler = new TimerScheduler();
         HttpConfiguration config = new HttpConfiguration();
@@ -139,6 +143,12 @@ public class ResponseTest
             @Override
             public void send(MetaData.Response info, boolean head, ByteBuffer content, boolean lastContent, Callback callback)
             {
+                if(BufferUtil.hasContent(content))
+                {
+                    BufferUtil.append(_content, content);
+                }
+            
+                
                 if (_channelError==null)
                     callback.succeeded();
                 else
@@ -172,6 +182,7 @@ public class ResponseTest
             {
                 return false;
             }
+            
         });
     }
 
@@ -364,6 +375,54 @@ public class ResponseTest
         assertTrue(response.toString().indexOf("charset=utf-8") > 0);
     }
 
+    @Test
+    public void testLocaleFormat() throws Exception
+    {
+        Response response = getResponse();
+
+        ContextHandler context = new ContextHandler();
+        context.addLocaleEncoding(Locale.ENGLISH.toString(), "ISO-8859-1");
+        context.addLocaleEncoding(Locale.ITALIAN.toString(), "ISO-8859-2");
+        response.getHttpChannel().getRequest().setContext(context.getServletContext());
+
+        response.setLocale(java.util.Locale.ITALIAN);
+        
+        PrintWriter out = response.getWriter();
+        
+        out.format("TestA1 %,.2f%n", 1234567.89);
+        out.format("TestA2 %,.2f%n", 1234567.89);
+        
+        out.format((java.util.Locale)null,"TestB1 %,.2f%n", 1234567.89);
+        out.format((java.util.Locale)null,"TestB2 %,.2f%n", 1234567.89);
+        
+        out.format(Locale.ENGLISH,"TestC1 %,.2f%n", 1234567.89);
+        out.format(Locale.ENGLISH,"TestC2 %,.2f%n", 1234567.89);
+        
+        out.format(Locale.ITALIAN,"TestD1 %,.2f%n", 1234567.89);
+        out.format(Locale.ITALIAN,"TestD2 %,.2f%n", 1234567.89);
+
+        out.close();
+        
+        /* Test A */
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestA1 1.234.567,89"));
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestA2 1.234.567,89"));
+        
+        /* Test B */
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestB1 1.234.567,89"));
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestB2 1.234.567,89"));
+        
+        /* Test C */
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestC1 1,234,567.89"));
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestC2 1,234,567.89"));
+        
+        /* Test D */
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestD1 1.234.567,89"));
+        Assert.assertThat(BufferUtil.toString(_content),Matchers.containsString("TestD2 1.234.567,89"));
+        
+        
+    }
+
+    
     @Test
     public void testContentTypeCharacterEncoding() throws Exception
     {

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java

@@ -514,7 +514,9 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory, Welc
             if ((_welcomeServlets || _welcomeExactServlets) && welcome_servlet==null)
             {
                 MappedResource<ServletHolder> entry=_servletHandler.getMappedServlet(welcome_in_context);
-                if (entry!=null && entry.getResource()!=_defaultHolder &&
+                @SuppressWarnings("ReferenceEquality")
+                boolean isDefaultHolder = (entry.getResource()!=_defaultHolder);
+                if (entry!=null && isDefaultHolder &&
                         (_welcomeServlets || (_welcomeExactServlets && entry.getPathSpec().getDeclaration().equals(welcome_in_context))))
                     welcome_servlet=welcome_in_context;
 

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java

@@ -1513,7 +1513,9 @@ public class ServletHandler extends ScopedHandler
         boolean found = false;
         for (ServletHolder s:_servlets)
         {
-            if (s == holder)
+            @SuppressWarnings("ReferenceEquality")
+            boolean foundServletHolder = (s == holder);
+            if (foundServletHolder)
                 found = true;
         }
         return found;

jetty-util/src/main/java/org/eclipse/jetty/util/Fields.java

@@ -253,7 +253,9 @@ public class Fields implements Iterable<Fields.Field>
 
         public boolean equals(Field that, boolean caseSensitive)
         {
-            if (this == that)
+            @SuppressWarnings("ReferenceEquality")
+            boolean isCurrentObject = (this == that);
+            if (isCurrentObject)
                 return true;
             if (that == null)
                 return false;

jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java

@@ -221,7 +221,7 @@ public class UrlEncoded extends MultiMap<String> implements Cloneable
         if (charset==null)
             charset=ENCODING;
 
-        if (charset==StandardCharsets.UTF_8)
+        if (StandardCharsets.UTF_8.equals(charset))
         {
             decodeUtf8To(content,0,content.length(),map);
             return;

jetty-util/src/main/java/org/eclipse/jetty/util/log/StdErrLog.java

@@ -211,7 +211,9 @@ public class StdErrLog extends AbstractLogger
      */
     public StdErrLog(String name, Properties props)
     {
-        if (props!=null && props!=Log.__props)
+        @SuppressWarnings("ReferenceEquality")
+        boolean sameObject = (props!=Log.__props);
+        if (props!=null && sameObject)
             Log.__props.putAll(props);
         _name = name == null?"":name;
         _abbrevname = condensePackageString(this._name);

jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java

@@ -416,7 +416,9 @@ public class FileResource extends Resource
             return false;
 
         FileResource f=(FileResource)o;
-        return f._file == _file || (null != _file && _file.equals(f._file));
+        @SuppressWarnings("ReferenceEquality")
+        boolean sameFile = f._file==_file;
+        return sameFile || (null != _file && _file.equals(f._file));
     }
 
     /* ------------------------------------------------------------ */

jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java

@@ -95,7 +95,9 @@ public abstract class Credential implements Serializable
      */
     protected static boolean stringEquals(String known, String unknown)
     {
-        if (known == unknown)
+        @SuppressWarnings("ReferenceEquality")
+        boolean sameObject = (known == unknown);
+        if (sameObject)
             return true;
         if (known == null || unknown == null)
             return false;

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClasspathPattern.java

@@ -696,9 +696,9 @@ public class ClasspathPattern extends AbstractSet<String>
                 LOG.debug("match {} from {} byName={} byLocation={} in {}",clazz,location,byName,byLocation,this);
             
             // Combine the tri-state match of both IncludeExclude Sets
-            boolean included = byName==Boolean.TRUE || byLocation==Boolean.TRUE
+            boolean included = Boolean.TRUE.equals(byName) || Boolean.TRUE.equals(byLocation)
                 || (byName==null && !_patterns.hasIncludes() && byLocation==null && !_locations.hasIncludes());
-            boolean excluded = byName==Boolean.FALSE || byLocation==Boolean.FALSE;
+            boolean excluded = Boolean.FALSE.equals(byName) || Boolean.FALSE.equals(byLocation);
             return included && !excluded;
         }
         catch (Exception e)
@@ -737,9 +737,9 @@ public class ClasspathPattern extends AbstractSet<String>
         }
 
         // Combine the tri-state match of both IncludeExclude Sets
-        boolean included = byName==Boolean.TRUE || byLocation==Boolean.TRUE
+        boolean included = Boolean.TRUE.equals(byName) || Boolean.TRUE.equals(byLocation)
             || (byName==null && !_patterns.hasIncludes() && byLocation==null && !_locations.hasIncludes());
-        boolean excluded = byName==Boolean.FALSE || byLocation==Boolean.FALSE;
+        boolean excluded = Boolean.FALSE.equals(byName) || Boolean.FALSE.equals(byLocation);
         return included && !excluded;
     }