Skip to content

Commit

Permalink
fix fortify high category issues
Browse files Browse the repository at this point in the history 
Fix issues in high category of fortify static analysis report

https://www.pivotaltracker.com/story/show/135822353

Change-Id: I883132359af9ee649646a42667a60fe3f7cd4a7c
  • Loading branch information
@ttddyy
ttddyy authored and Gerrit Code Review committed Jan 12, 2017
1 parent 830c9a8 commit d65ba0c
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 94 deletions.
13 changes: 4 additions & 9 deletions xenon-common/src/main/java/com/vmware/xenon/common/ReflectionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,12 @@ public static Field getField(Class<?> clazz, String name) {
.collect(toMap(Field::getName, identity()))
);

Field field = fieldMap.get(name);
if (field == null) {
return null;
}

if (!field.isAccessible()) {
synchronized (field) {
return fieldMap.computeIfPresent(name, (k, field) -> {
if (!field.isAccessible()) {
field.setAccessible(true);
}
}
return field;
return field;
});
}

}
11 changes: 1 addition & 10 deletions xenon-common/src/main/java/com/vmware/xenon/common/RequestRouter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,7 @@ public static class RequestBodyMatcher<T> implements Predicate<Operation> {
public RequestBodyMatcher(Class<T> typeParameterClass, String fieldName, Object fieldValue) {
this.typeParameterClass = typeParameterClass;

Field f;
try {
f = typeParameterClass.getField(fieldName);
//PODO's fields are public, just in case
f.setAccessible(true);
} catch (ReflectiveOperationException e) {
f = null;
}
this.field = f;

this.field = ReflectionUtils.getField(typeParameterClass, fieldName);
this.fieldValue = fieldValue;
}

Expand Down
72 changes: 31 additions & 41 deletions xenon-common/src/main/java/com/vmware/xenon/common/StatefulService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -755,54 +755,44 @@ private void handleRequestCompletion(Operation op, Throwable e) {
return;
}

boolean processPending = true;
try {
if (op.getAction() == Action.DELETE && op.getTransactionId() == null
&& handleDeleteCompletion(op)) {
processPending = false;
return;
}

if (op.getAction() == Action.OPTIONS) {
handleOptionsCompletion(op);
return;
}
if (op.getAction() == Action.DELETE && op.getTransactionId() == null && handleDeleteCompletion(op)) {
return;
}

if (isStateUpdated && linkedState != null) {
// defense in depth: conform state so core fields are properly set
if (linkedState.documentDescription != null) {
linkedState.documentDescription = null;
}
if (op.getAction() == Action.OPTIONS) {
handleOptionsCompletion(op);
processPending(op);
return;
}

linkedState.documentSelfLink = this.context.selfLink;
linkedState.documentUpdateAction = op.getAction().name();
if (linkedState.documentKind == null) {
linkedState.documentKind = Utils.buildKind(this.context.stateType);
}
if (op.getTransactionId() != null && linkedState.documentTransactionId == null) {
// reset transaction id in case it got overwritten by the service handler;
// unless it's a transaction control op
if (op.getRequestHeader(Operation.TRANSACTION_HEADER) == null) {
linkedState.documentTransactionId = op.getTransactionId();
}
}
if (isStateUpdated && linkedState != null) {
// defense in depth: conform state so core fields are properly set
if (linkedState.documentDescription != null) {
linkedState.documentDescription = null;
}

if (processCompletionStageReplicationProposal(op)) {
processPending = false;
return;
linkedState.documentSelfLink = this.context.selfLink;
linkedState.documentUpdateAction = op.getAction().name();
if (linkedState.documentKind == null) {
linkedState.documentKind = Utils.buildKind(this.context.stateType);
}
if (op.getTransactionId() != null && linkedState.documentTransactionId == null) {
// reset transaction id in case it got overwritten by the service handler;
// unless it's a transaction control op
if (op.getRequestHeader(Operation.TRANSACTION_HEADER) == null) {
linkedState.documentTransactionId = op.getTransactionId();
}

// next stage will process pending operations, disable finally clause from
// duplicating work
processPending = false;
processCompletionStageIndexing(op);
} else {
processCompletionStagePublishAndComplete(op);
}
} finally {
if (!processPending) {

if (processCompletionStageReplicationProposal(op)) {
return;
}

// next stage will process pending operations, disable finally clause from
// duplicating work
processCompletionStageIndexing(op);
} else {
processCompletionStagePublishAndComplete(op);
processPending(op);
}
}
Expand Down
14 changes: 12 additions & 2 deletions xenon-common/src/main/java/com/vmware/xenon/common/UtilityService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

package com.vmware.xenon.common;

import static com.vmware.xenon.common.Service.Action.DELETE;
import static com.vmware.xenon.common.Service.Action.POST;

import java.io.NotActiveException;
import java.io.UnsupportedEncodingException;
import java.net.URI;
Expand Down Expand Up @@ -147,15 +150,22 @@ private void handleSubscriptionsRequest(Operation op) {
}

ServiceSubscriber body = null;
if (op.hasBody()) {

// validate and populate body for POST & DELETE
Action action = op.getAction();
if (action == POST || action == DELETE) {
if (!op.hasBody()) {
op.fail(new IllegalStateException("body is required"));
return;
}
body = op.getBody(ServiceSubscriber.class);
if (body.reference == null) {
op.fail(new IllegalArgumentException("reference is required"));
return;
}
}

switch (op.getAction()) {
switch (action) {
case POST:
// synchronize to avoid concurrent modification during serialization for GET
synchronized (this.subscriptions) {
Expand Down
8 changes: 4 additions & 4 deletions xenon-common/src/main/java/com/vmware/xenon/common/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,8 @@ public static String validateServiceOption(EnumSet<ServiceOption> options,
return null;
}

EnumSet<ServiceOption> reqs = null;
EnumSet<ServiceOption> antiReqs = null;
EnumSet<ServiceOption> reqs = EnumSet.noneOf(ServiceOption.class);
EnumSet<ServiceOption> antiReqs = EnumSet.noneOf(ServiceOption.class);
switch (option) {
case CONCURRENT_UPDATE_HANDLING:
antiReqs = EnumSet.of(ServiceOption.OWNER_SELECTION,
Expand Down Expand Up @@ -657,11 +657,11 @@ public static String validateServiceOption(EnumSet<ServiceOption> options,
break;
}

if (reqs == null && antiReqs == null) {
if (reqs.isEmpty() && antiReqs.isEmpty()) {
return null;
}

if (reqs != null) {
if (!reqs.isEmpty()) {
EnumSet<ServiceOption> missingReqs = EnumSet.noneOf(ServiceOption.class);
for (ServiceOption r : reqs) {
if (!options.contains(r)) {
Expand Down
39 changes: 14 additions & 25 deletions xenon-common/src/main/java/com/vmware/xenon/services/common/NodeGroupService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -647,7 +648,7 @@ public void handleMaintenance(Operation maint) {
peersToProbe = Math.min(localState.nodes.size() - 1, peersToProbe);

AtomicInteger remaining = new AtomicInteger(peersToProbe);
NodeState[] randomizedPeers = shuffleGroupMembers(localState);
List<NodeState> randomizedPeers = shuffleGroupMembers(localState);
NodeState localNode = localState.nodes.get(getHost().getId());
localNode.documentUpdateTimeMicros = Utils.getNowMicrosUtc();
localNode.groupReference = UriUtils.buildPublicUri(getHost(), getSelfLink());
Expand Down Expand Up @@ -763,17 +764,16 @@ public void handleGossipPatchCompletion(Operation maint, Operation patch, Throwa

} finally {
int r = remaining.decrementAndGet();
if (r != 0) {
return;
}

// to merge updated state, issue a self PATCH. It contains NodeState entries for every
// peer node we just talked to
sendRequest(Operation.createPatch(getUri())
.setBodyNoCloning(patchBody));
if (r <= 0) {
// to merge updated state, issue a self PATCH. It contains NodeState entries for every
// peer node we just talked to
sendRequest(Operation.createPatch(getUri())
.setBodyNoCloning(patchBody));

maint.complete();
maint.complete();
}
}

}

/**
Expand Down Expand Up @@ -949,21 +949,10 @@ private void mergeRemoteAndLocalMembership(
}
}

public NodeState[] shuffleGroupMembers(NodeGroupState localState) {
// randomize the list of peers and place them in array. Then pick the first N peers to
// probe. We can probably come up with a single pass, probabilistic approach
// but this works for now and is relatively cheap even for groups with thousands of members
NodeState[] randomizedPeers = new NodeState[localState.nodes.size()];
localState.nodes.values().toArray(randomizedPeers);

ThreadLocalRandom random = ThreadLocalRandom.current();
for (int i = randomizedPeers.length - 1; i > 0; i--) {
int index = random.nextInt(i + 1);
NodeState t = randomizedPeers[index];
randomizedPeers[index] = randomizedPeers[i];
randomizedPeers[i] = t;
}
return randomizedPeers;
private List<NodeState> shuffleGroupMembers(NodeGroupState localState) {
List<NodeState> peers = new ArrayList<>(localState.nodes.values());
Collections.shuffle(peers, ThreadLocalRandom.current());
return peers;
}

private NodeGroupState getStateFromBody(Operation o) {
Expand Down
8 changes: 5 additions & 3 deletions xenon-common/src/main/java/com/vmware/xenon/services/common/ServiceContextIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,11 @@ void handleSerializePut(Operation put, ServiceRuntimeContext s) {
put.fail(e);
return;
} finally {
try {
output.close();
} catch (IOException e) {
if (output != null) {
try {
output.close();
} catch (IOException e) {
}
}
}

Expand Down

0 comments on commit d65ba0c

Please sign in to comment.