Skip to content

Commit

Permalink
Fix #6868 Fix copy constructors (#6872)
Browse files Browse the repository at this point in the history 
* Fix #6868 Fix copy constructors

Fixed copy constructors found by static code analysis

Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
@gregw
gregw committed Sep 20, 2021
1 parent 4793092 commit f398da7
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 32 deletions.
1 change: 1 addition & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
View file
Expand Up @@ -144,6 +144,7 @@ public HttpConfiguration(HttpConfiguration config)
_responseCookieCompliance = config._responseCookieCompliance;
_notifyRemoteAsyncErrors = config._notifyRemoteAsyncErrors;
_relativeRedirectAllowed = config._relativeRedirectAllowed;
_uriCompliance = config._uriCompliance;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClassMatcher.java
View file
Expand Up @@ -496,6 +496,7 @@ public ClassMatcher()
{
}

@SuppressWarnings("CopyConstructorMissesField")
public ClassMatcher(ClassMatcher patterns)
{
if (patterns != null)
Expand Down Expand Up @@ -676,7 +677,7 @@ private void addAll(String[] classes)
*/
public String[] getPatterns()
{
return toArray(new String[_entries.size()]);
return toArray(new String[0]);
}

/**
Expand Down
70 changes: 39 additions & 31 deletions jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ClassMatcherTest.java
View file
Expand Up @@ -27,6 +27,7 @@
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand Down Expand Up @@ -119,6 +120,13 @@ public void testMatchAll()
assertTrue(_pattern.match("org.example.Anything$Else"));
}

@Test
public void testCopy()
{
ClassMatcher copy = new ClassMatcher(_pattern);
assertThat(copy.toString(), is(_pattern.toString()));
}

@Test
public void testMatchFundamentalExcludeSpecific()
{
Expand All @@ -145,24 +153,24 @@ public void testIncludedLocations() throws Exception

ClassMatcher pattern = new ClassMatcher();
pattern.include("something");
assertThat(pattern.match(String.class), Matchers.is(false));
assertThat(pattern.match(Test.class), Matchers.is(false));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(false));
assertThat(pattern.match(String.class), is(false));
assertThat(pattern.match(Test.class), is(false));
assertThat(pattern.match(ClassMatcherTest.class), is(false));

// Add directory for both JVM classes
pattern.include(locString.toASCIIString());

// Add jar for individual class and classes directory
pattern.include(locJunit.toString(), locTest.toString());

assertThat(pattern.match(String.class), Matchers.is(true));
assertThat(pattern.match(Test.class), Matchers.is(true));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(true));
assertThat(pattern.match(String.class), is(true));
assertThat(pattern.match(Test.class), is(true));
assertThat(pattern.match(ClassMatcherTest.class), is(true));

pattern.add("-java.lang.String");
assertThat(pattern.match(String.class), Matchers.is(false));
assertThat(pattern.match(Test.class), Matchers.is(true));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(true));
assertThat(pattern.match(String.class), is(false));
assertThat(pattern.match(Test.class), is(true));
assertThat(pattern.match(ClassMatcherTest.class), is(true));
}

@SuppressWarnings("restriction")
Expand All @@ -183,24 +191,24 @@ public void testIncludedLocationsOrModule() throws Exception

ClassMatcher pattern = new ClassMatcher();
pattern.include("something");
assertThat(pattern.match(String.class), Matchers.is(false));
assertThat(pattern.match(Test.class), Matchers.is(false));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(false));
assertThat(pattern.match(String.class), is(false));
assertThat(pattern.match(Test.class), is(false));
assertThat(pattern.match(ClassMatcherTest.class), is(false));

// Add module for all JVM base classes
pattern.include("jrt:/java.base");

// Add jar for individual class and classes directory
pattern.include(locJunit.toString(), locTest.toString());

assertThat(pattern.match(String.class), Matchers.is(true));
assertThat(pattern.match(Test.class), Matchers.is(true));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(true));
assertThat(pattern.match(String.class), is(true));
assertThat(pattern.match(Test.class), is(true));
assertThat(pattern.match(ClassMatcherTest.class), is(true));

pattern.add("-java.lang.String");
assertThat(pattern.match(String.class), Matchers.is(false));
assertThat(pattern.match(Test.class), Matchers.is(true));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(true));
assertThat(pattern.match(String.class), is(false));
assertThat(pattern.match(Test.class), is(true));
assertThat(pattern.match(ClassMatcherTest.class), is(true));
}

@SuppressWarnings("restriction")
Expand All @@ -224,19 +232,19 @@ public void testExcludeLocationsOrModule() throws Exception
// include everything
pattern.include(".");

assertThat(pattern.match(String.class), Matchers.is(true));
assertThat(pattern.match(Test.class), Matchers.is(true));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(true));
assertThat(pattern.match(String.class), is(true));
assertThat(pattern.match(Test.class), is(true));
assertThat(pattern.match(ClassMatcherTest.class), is(true));

// Add directory for both JVM classes
pattern.exclude("jrt:/java.base/");

// Add jar for individual class and classes directory
pattern.exclude(locJunit.toString(), locTest.toString());

assertThat(pattern.match(String.class), Matchers.is(false));
assertThat(pattern.match(Test.class), Matchers.is(false));
assertThat(pattern.match(ClassMatcherTest.class), Matchers.is(false));
assertThat(pattern.match(String.class), is(false));
assertThat(pattern.match(Test.class), is(false));
assertThat(pattern.match(ClassMatcherTest.class), is(false));
}

@Test
Expand All @@ -248,33 +256,33 @@ public void testWithNullLocation() throws Exception
IncludeExcludeSet<Entry, URI> locations = new IncludeExcludeSet<>(ByLocationOrModule.class);

//Test no name or location includes or excludes - should match
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), Matchers.is(true));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), is(true));

names.include(matcher.newEntry("a.b.", true));
names.exclude(matcher.newEntry("d.e.", false));

//Test explicit include by name no locations - should match
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), Matchers.is(true));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), is(true));

//Test explicit exclude by name no locations - should not match
assertThat(ClassMatcher.combine(names, "d.e.f", locations, NULL_SUPPLIER), Matchers.is(false));
assertThat(ClassMatcher.combine(names, "d.e.f", locations, NULL_SUPPLIER), is(false));

//Test include by name with location includes - should match
locations.include(matcher.newEntry("file:/foo/bar", true));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), Matchers.is(true));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), is(true));

//Test include by name but with location exclusions - should not match
locations.clear();
locations.exclude(matcher.newEntry("file:/high/low", false));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), Matchers.is(false));
assertThat(ClassMatcher.combine(names, "a.b.c", locations, NULL_SUPPLIER), is(false));

//Test neither included or excluded by name, but with location exclusions - should not match
assertThat(ClassMatcher.combine(names, "g.b.r", locations, NULL_SUPPLIER), Matchers.is(false));
assertThat(ClassMatcher.combine(names, "g.b.r", locations, NULL_SUPPLIER), is(false));

//Test neither included nor excluded by name, but with location inclusions - should not match
locations.clear();
locations.include(matcher.newEntry("file:/foo/bar", true));
assertThat(ClassMatcher.combine(names, "g.b.r", locations, NULL_SUPPLIER), Matchers.is(false));
assertThat(ClassMatcher.combine(names, "g.b.r", locations, NULL_SUPPLIER), is(false));
}

@Test
Expand Down

0 comments on commit f398da7

Please sign in to comment.