Apache
/
hbase
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HBASE-7443 More findbugs fixes 

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1426511 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
nkeywal 2012-12-28 13:56:15 +00:00
parent 3e600deba2
commit 92c98a76d1
17 changed files with 116 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
dev-support/findbugs-exclude.xml
View File

@ -115,7 +115,6 @@
</Match>
<Match>
<Class name="org.apache.hadoop.hbase.filter.ParseConstants"/>
<!--
A mutable static field could be changed by malicious code or by accident. The field could be
made package protected to avoid this vulnerability.
@ -125,8 +124,12 @@
Warning is not wrong, but difficult to avoid...
!-->
<Bug pattern="PKGPROTECT"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Bug pattern="MS_OOI_PKGPROTECT"/>
</Match>
<Match>
<!--
@ -195,7 +198,42 @@
<Bug pattern="DM_EXIT"/>
</Match>
<Match>
<!--
This method returns a value that is not checked. The return value should be checked since
it can indicate an unusual or unexpected function execution. For example, the
File.delete() method returns false if the file could not be successfully deleted
(rather than throwing an Exception). If you don't check the result, you won't notice
if the method invocation signals unexpected behavior by returning an atypical return
value.
It's so bad that the reviews will catch all the wrong cases.
!-->
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE"/>
</Match>
<Match>
<Bug pattern="RV_RETURN_VALUE_IGNORED_INFERRED"/>
</Match>
<Match>
<!--
This method contains a redundant check of a known non-null value against the constant null.
Most of the time we're securing ourselves, does no much harm.
!-->
<Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
</Match>
<Match>
<!--
A final static field references an array and can be accessed by malicious code or by
accident from another package. This code can freely modify the contents of the array.
We've got this all over the place... Cloning the array by security is not a general
solution from a performance point of view
!-->
<Bug pattern="MS_MUTABLE_ARRAY"/>
</Match>
</FindBugsFilter>

16
hbase-server/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
View File

@ -164,14 +164,6 @@ public class HRegionInfo implements Comparable<HRegionInfo> {
return encodedRegionName;
}
/** HRegionInfo for root region */
public static final HRegionInfo ROOT_REGIONINFO =
new HRegionInfo(0L, Bytes.toBytes("-ROOT-"));
/** HRegionInfo for first meta region */
public static final HRegionInfo FIRST_META_REGIONINFO =
new HRegionInfo(1L, Bytes.toBytes(".META."));
private byte [] endKey = HConstants.EMPTY_BYTE_ARRAY;
// This flag is in the parent of a split while the parent is still referenced
// by daughter regions. We USED to set this flag when we disabled a table
@ -191,6 +183,14 @@ public class HRegionInfo implements Comparable<HRegionInfo> {
// Current TableName
private byte[] tableName = null;
/** HRegionInfo for root region */
public static final HRegionInfo ROOT_REGIONINFO =
new HRegionInfo(0L, Bytes.toBytes("-ROOT-"));
/** HRegionInfo for first meta region */
public static final HRegionInfo FIRST_META_REGIONINFO =
new HRegionInfo(1L, Bytes.toBytes(".META."));
private void setHashCode() {
int result = Arrays.hashCode(this.regionName);
result ^= this.regionId;

1
hbase-server/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
View File

@ -115,7 +115,6 @@ public class MetaReader {
public boolean visit(Result r) throws IOException {
if (r == null || r.isEmpty()) return true;
Pair<HRegionInfo, ServerName> region = HRegionInfo.getHRegionInfoAndServerName(r);
if (region == null) return true;
HRegionInfo hri = region.getFirst();
if (hri == null) return true;
if (hri.getTableNameAsString() == null) return true;

21
hbase-server/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
View File

@ -410,7 +410,7 @@ public class HConnectionManager {
*
*/
public static class HConnectionKey {
public static String[] CONNECTION_PROPERTIES = new String[] {
final static String[] CONNECTION_PROPERTIES = new String[] {
HConstants.ZOOKEEPER_QUORUM, HConstants.ZOOKEEPER_ZNODE_PARENT,
HConstants.ZOOKEEPER_CLIENT_PORT,
HConstants.ZOOKEEPER_RECOVERABLE_WAITTIME,
@ -464,6 +464,9 @@ public class HConnectionManager {
return result;
}
@edu.umd.cs.findbugs.annotations.SuppressWarnings (value="ES_COMPARING_STRINGS_WITH_EQ",
justification="Optimization")
@Override
public boolean equals(Object obj) {
if (this == obj)
@ -489,6 +492,7 @@ public class HConnectionManager {
for (String property : CONNECTION_PROPERTIES) {
String thisValue = this.properties.get(property);
String thatValue = that.properties.get(property);
//noinspection StringEquality
if (thisValue == thatValue) {
continue;
}
@ -731,11 +735,13 @@ public class HConnectionManager {
/**
* Create a master, retries if necessary.
*/
@edu.umd.cs.findbugs.annotations.SuppressWarnings (value="SWL_SLEEP_WITH_LOCK_HELD")
private MasterProtocol createMasterWithRetries(
MasterProtocolState masterProtocolState) throws MasterNotRunningException {
// The lock must be at the beginning to prevent multiple master creation
// (and leaks) in a multithread context
synchronized (this.masterAndZKLock) {
Exception exceptionCaught = null;
MasterProtocol master = null;
@ -1309,7 +1315,7 @@ public class HConnectionManager {
byte [] startKey = location.getRegionInfo().getStartKey();
Map<byte [], HRegionLocation> tableLocations =
getTableLocations(tableName);
boolean hasNewCache = false;
boolean hasNewCache;
synchronized (this.cachedRegionLocations) {
cachedServers.add(location.getHostnamePort());
hasNewCache = (tableLocations.put(startKey, location) == null);
@ -1459,7 +1465,7 @@ public class HConnectionManager {
/**
* Creates a Chore thread to check the connections to master & zookeeper
* and close them when they reach their closing time (
* {@link #MasterProtocolState.keepAliveUntil} and
* {@link MasterProtocolState#keepAliveUntil} and
* {@link #keepZooKeeperWatcherAliveUntil}). Keep alive time is
* managed by the release functions and the variable {@link #keepAlive}
*/
@ -1614,7 +1620,7 @@ public class HConnectionManager {
@Override
public MasterAdminProtocol getMasterAdmin() throws MasterNotRunningException {
return getKeepAliveMasterAdmin();
};
}
@Override
public MasterMonitorProtocol getMasterMonitor() throws MasterNotRunningException {
@ -1759,9 +1765,10 @@ public class HConnectionManager {
private void updateCachedLocations(final HRegionLocation hrl, final byte[] tableName,
Row row, final Object exception) {
if ((row == null || tableName == null) && hrl == null){
LOG.warn ("Coding error, see method javadoc. row="+row+", tableName="+
Bytes.toString(tableName)+", hrl="+hrl);
if ((row == null || tableName == null) && hrl == null) {
LOG.warn("Coding error, see method javadoc. row=" + (row == null ? "null" : row) +
", tableName=" + (tableName == null ? "null" : Bytes.toString(tableName) +
", hrl= null"));
return;
}

7
hbase-server/src/main/java/org/apache/hadoop/hbase/filter/ByteArrayComparable.java
View File

@ -75,11 +75,10 @@ public abstract class ByteArrayComparable implements Comparable<byte[]> {
* @return true if and only if the fields of the comparator that are serialized
* are equal to the corresponding fields in other. Used for testing.
*/
boolean areSerializedFieldsEqual(ByteArrayComparable o) {
if (o == this) return true;
if (!(o instanceof ByteArrayComparable)) return false;
boolean areSerializedFieldsEqual(ByteArrayComparable other) {
if (other == this) return true;
return Bytes.equals(this.getValue(), o.getValue());
return Bytes.equals(this.getValue(), other.getValue());
}
@Override

2
hbase-server/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java
View File

@ -156,7 +156,7 @@ public abstract class CompareFilter extends FilterBase {
/**
*
* @param other
* @param o
* @return true if and only if the fields of the filter that are serialized
* are equal to the corresponding fields in other. Used for testing.
*/

4
hbase-server/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java
View File

@ -261,10 +261,12 @@ public class DependentColumnFilter extends CompareFilter {
}
/**
* @param other
* @param o
* @return true if and only if the fields of the filter that are serialized
* are equal to the corresponding fields in other. Used for testing.
*/
@edu.umd.cs.findbugs.annotations.SuppressWarnings(
value="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE")
boolean areSerializedFieldsEqual(Filter o) {
if (o == this) return true;
if (!(o instanceof DependentColumnFilter)) return false;

18
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
View File

@ -418,16 +418,14 @@ public class HBaseClient {
return null;
}
UserInformation.Builder userInfoPB = UserInformation.newBuilder();
if (ugi != null) {
if (authMethod == AuthMethod.KERBEROS) {
// Send effective user for Kerberos auth
userInfoPB.setEffectiveUser(ugi.getUserName());
} else if (authMethod == AuthMethod.SIMPLE) {
//Send both effective user and real user for simple auth
userInfoPB.setEffectiveUser(ugi.getUserName());
if (ugi.getRealUser() != null) {
userInfoPB.setRealUser(ugi.getRealUser().getUserName());
}
if (authMethod == AuthMethod.KERBEROS) {
// Send effective user for Kerberos auth
userInfoPB.setEffectiveUser(ugi.getUserName());
} else if (authMethod == AuthMethod.SIMPLE) {
//Send both effective user and real user for simple auth
userInfoPB.setEffectiveUser(ugi.getUserName());
if (ugi.getRealUser() != null) {
userInfoPB.setRealUser(ugi.getRealUser().getUserName());
}
}
return userInfoPB.build();

2
hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java
View File

@ -426,6 +426,8 @@ public class HFileOutputFormat extends FileOutputFormat<ImmutableBytesWritable,
* @throws IOException
* on failure to read column family descriptors
*/
@edu.umd.cs.findbugs.annotations.SuppressWarnings(
value="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE")
static void configureCompression(HTable table, Configuration conf) throws IOException {
StringBuilder compressionConfigValue = new StringBuilder();
HTableDescriptor tableDescriptor = table.getTableDescriptor();

2
hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
View File

@ -610,7 +610,7 @@ public class AssignmentManager extends ZooKeeperListener {
*/
private void handleRegion(final RegionTransition rt, int expectedVersion) {
if (rt == null) {
LOG.warn("Unexpected NULL input " + rt);
LOG.warn("Unexpected NULL input for RegionTransition rt");
return;
}
final ServerName sn = rt.getServerName();

6
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
View File

@ -380,7 +380,7 @@ public class ServerManager {
public double getAverageLoad() {
int totalLoad = 0;
int numServers = 0;
double averageLoad = 0.0;
double averageLoad;
for (ServerLoad sl: this.onlineServers.values()) {
numServers++;
totalLoad += sl.getNumberOfRegions();
@ -680,7 +680,7 @@ public class ServerManager {
*/
private AdminProtocol getServerConnection(final ServerName sn)
throws IOException {
AdminProtocol admin = this.serverConnections.get(sn.toString());
AdminProtocol admin = this.serverConnections.get(sn);
if (admin == null) {
LOG.debug("New connection to " + sn.toString());
admin = this.connection.getAdmin(sn.getHostname(), sn.getPort());
@ -886,7 +886,7 @@ public class ServerManager {
* To clear any dead server with same host name and port of any online server
*/
void clearDeadServersWithSameHostNameAndPortOfOnlineServer() {
ServerName sn = null;
ServerName sn;
for (ServerName serverName : getOnlineServersList()) {
while ((sn = ServerName.
findServerWithSameHostnamePort(this.deadservers, serverName)) != null) {

5
hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/LogMonitoring.java
View File

@ -79,17 +79,18 @@ public abstract class LogMonitoring {
private static void dumpTailOfLog(File f, PrintWriter out, long tailKb)
throws IOException {
FileInputStream fis = new FileInputStream(f);
BufferedReader r = null;
try {
FileChannel channel = fis.getChannel();
channel.position(Math.max(0, channel.size() - tailKb*1024));
BufferedReader r = new BufferedReader(
new InputStreamReader(fis));
r = new BufferedReader(new InputStreamReader(fis));
r.readLine(); // skip the first partial line
String line;
while ((line = r.readLine()) != null) {
out.println(line);
}
} finally {
if (r != null) IOUtils.closeStream(r);
IOUtils.closeStream(fis);
}
}

2
hbase-server/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
View File

@ -638,7 +638,7 @@ public class StorageClusterStatusModel
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append(String.format("%d live servers, %d dead servers, " +
"%.4f average load\n\n", liveNodes.size(), deadNodes.size(),
"%.4f average load%n%n", liveNodes.size(), deadNodes.size(),
averageLoad));
if (!liveNodes.isEmpty()) {
sb.append(liveNodes.size());

4
hbase-server/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java
View File

@ -32,7 +32,7 @@ import org.apache.hadoop.security.authorize.ServiceAuthorizationManager;
* protocol interfaces to hbase-policy.xml entries.
*/
public class HBasePolicyProvider extends PolicyProvider {
protected static Service[] services = {
protected final static Service[] services = {
new Service("security.client.protocol.acl", ClientProtocol.class),
new Service("security.client.protocol.acl", AdminProtocol.class),
new Service("security.admin.protocol.acl", MasterMonitorProtocol.class),
@ -51,7 +51,7 @@ public class HBasePolicyProvider extends PolicyProvider {
conf.set("hadoop.policy.file", "hbase-policy.xml");
if (conf.getBoolean(
ServiceAuthorizationManager.SERVICE_AUTHORIZATION_CONFIG, false)) {
authManager.refresh(conf, new HBasePolicyProvider());
ServiceAuthorizationManager.refresh(conf, new HBasePolicyProvider());
}
}
}

18
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
View File

@ -149,8 +149,7 @@ public class AccessController extends BaseRegionObserver
}
public String toString() {
return new StringBuilder("AuthResult")
.append(toContextString()).toString();
return "AuthResult" + toContextString();
}
public static AuthResult allow(String request, String reason, User user, Permission.Action action,
@ -235,7 +234,7 @@ public class AccessController extends BaseRegionObserver
byte[] serialized = AccessControlLists.writePermissionsAsBytes(perms, conf);
zkw.writeToZookeeper(tableName, serialized);
} catch (IOException ex) {
LOG.error("Failed updating permissions mirror for '" + tableName + "'", ex);
LOG.error("Failed updating permissions mirror for '" + Bytes.toString(tableName) + "'", ex);
}
}
}
@ -455,7 +454,7 @@ public class AccessController extends BaseRegionObserver
logResult(result);
if (!result.isAllowed()) {
StringBuffer sb = new StringBuffer("");
StringBuilder sb = new StringBuilder("");
if ((families != null && families.size() > 0)) {
for (byte[] familyName : families.keySet()) {
if (sb.length() != 0) {
@ -785,7 +784,6 @@ public class AccessController extends BaseRegionObserver
final HRegion region = env.getRegion();
if (region == null) {
LOG.error("NULL region from RegionCoprocessorEnvironment in preOpen()");
return;
} else {
HRegionInfo regionInfo = region.getRegionInfo();
if (isSpecialTable(regionInfo)) {
@ -849,8 +847,10 @@ public class AccessController extends BaseRegionObserver
public void preGetClosestRowBefore(final ObserverContext<RegionCoprocessorEnvironment> c,
final byte [] row, final byte [] family, final Result result)
throws IOException {
assert family != null;
//noinspection PrimitiveArrayArgumentToVariableArgMethod
requirePermission("getClosestRowBefore", Permission.Action.READ, c.getEnvironment(),
(family != null ? Lists.newArrayList(family) : null));
Lists.newArrayList(family));
}
@Override
@ -1258,9 +1258,9 @@ public class AccessController extends BaseRegionObserver
private boolean isSpecialTable(HRegionInfo regionInfo) {
byte[] tableName = regionInfo.getTableName();
return tableName.equals(AccessControlLists.ACL_TABLE_NAME)
|| tableName.equals(Bytes.toBytes("-ROOT-"))
|| tableName.equals(Bytes.toBytes(".META."));
return Arrays.equals(tableName, AccessControlLists.ACL_TABLE_NAME)
|| Arrays.equals(tableName, Bytes.toBytes("-ROOT-"))
|| Arrays.equals(tableName, Bytes.toBytes(".META."));
}
@Override

2
hbase-server/src/main/java/org/apache/hadoop/hbase/tool/Canary.java
View File

@ -173,7 +173,7 @@ public final class Canary implements Tool {
}
private void printUsageAndExit() {
System.err.printf("Usage: bin/hbase %s [opts] [table 1 [table 2...]]\n", getClass().getName());
System.err.printf("Usage: bin/hbase %s [opts] [table 1 [table 2...]]%n", getClass().getName());
System.err.println(" where [opts] are:");
System.err.println(" -help Show this help and exit.");
System.err.println(" -daemon Continuous check at defined intervals.");

19
pom.xml
View File

@ -506,7 +506,7 @@
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>${findbugs.version}</version>
<version>${findbugs-maven-plugin.version}</version>
<configuration>
<excludeFilterFile>${project.basedir}/../dev-support/findbugs-exclude.xml</excludeFilterFile>
<findbugsXmlOutput>true</findbugsXmlOutput>
@ -866,7 +866,8 @@
<maven.assembly.version>2.3</maven.assembly.version>
<maven.antrun.version>1.6</maven.antrun.version>
<jamon.plugin.version>2.3.4</jamon.plugin.version>
<findbugs.version>2.5.2</findbugs.version>
<findbugs-maven-plugin.version>2.5.2</findbugs-maven-plugin.version>
<findbugs.version>2.0.1</findbugs.version> <!-- as the plugin version for safety -->
<maven.site.version>3.1</maven.site.version>
<javadoc.version>2.9</javadoc.version>
<maven.resources.plugin.version>2.5</maven.resources.plugin.version>
@ -1244,6 +1245,20 @@
</dependencyManagement>
<!-- Dependencies needed by subprojects -->
<dependencies>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>annotations</artifactId>
<version>${findbugs.version}</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>${findbugs.version}</version>
<scope>compile</scope>
</dependency>
<!-- Test dependencies -->
<dependency>
<groupId>junit</groupId>