Re-check use of "useLoc" once more.

Fixes:
* On passable-violations useLoc might have been passed as newTo.

Reduce potential for accidental future misuse:
* Call useLoc.setWorld(null) as late as possible.
* Do not reset world of MoveInfo.useLoc in MoveInfo.set.
This commit is contained in:
asofold 2014-05-15 10:06:47 +02:00
parent e7f292ebe0
commit 521b4feb25
5 changed files with 48 additions and 22 deletions

View File

@ -134,7 +134,6 @@ public class BlockPlaceListener extends CheckListener {
if (!cancelled && reach.isEnabled(player) && reach.check(player, block, data, cc)) { if (!cancelled && reach.isEnabled(player) && reach.check(player, block, data, cc)) {
cancelled = true; cancelled = true;
} }
useLoc.setWorld(null);
// Direction check. // Direction check.
if (!cancelled && direction.isEnabled(player) && direction.check(player, block, blockAgainst, data, cc)) { if (!cancelled && direction.isEnabled(player) && direction.check(player, block, blockAgainst, data, cc)) {
@ -150,6 +149,8 @@ public class BlockPlaceListener extends CheckListener {
if (cancelled) { if (cancelled) {
event.setCancelled(cancelled); event.setCancelled(cancelled);
} }
// Cleanup
// Reminder(currently unused): useLoc.setWorld(null);
} }
@EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true)

View File

@ -42,11 +42,14 @@ public class Angle extends Check {
boolean cancel = false; boolean cancel = false;
// Remove the old locations from the map. // Remove the old locations from the map.
for (final long time : new TreeMap<Long, Location>(data.angleHits).navigableKeySet()) for (final long time : new TreeMap<Long, Location>(data.angleHits).navigableKeySet()) {
if (System.currentTimeMillis() - time > 1000L) if (System.currentTimeMillis() - time > 1000L) {
data.angleHits.remove(time); data.angleHits.remove(time);
}
}
// Add the new location to the map. // Add the new location to the map.
// TODO: Alter method to store something less fat.
data.angleHits.put(System.currentTimeMillis(), player.getLocation()); // This needs to be a copy at present. data.angleHits.put(System.currentTimeMillis(), player.getLocation()); // This needs to be a copy at present.
// Not enough data to calculate deltas. // Not enough data to calculate deltas.

View File

@ -13,11 +13,12 @@ import fr.neatmonster.nocheatplus.utilities.PlayerLocation;
* *
*/ */
public class MoveInfo { public class MoveInfo {
/** For temporary use. Might need cloning for passing to external API. Only use after calling MoveInfo.set! */
public final Location useLoc = new Location(null, 0, 0, 0);
public final BlockCache cache; public final BlockCache cache;
public final PlayerLocation from; public final PlayerLocation from;
public final PlayerLocation to; public final PlayerLocation to;
/** For temporary use. Might need cloning for passing to external API. */
public final Location useLoc = new Location(null, 0, 0, 0);
public MoveInfo(final MCAccess mcAccess){ public MoveInfo(final MCAccess mcAccess){
cache = mcAccess.getBlockCache(null); cache = mcAccess.getBlockCache(null);
@ -26,26 +27,31 @@ public class MoveInfo {
} }
/** /**
* Demands at least setting from. * Initialize from, and if given to. Note that useLoc is left untouched (!).
* @param player * @param player
* @param from * @param from Must not be null.
* @param to * @param to Can be null.
* @param yOnGround * @param yOnGround
*/ */
public final void set(final Player player, final Location from, final Location to, final double yOnGround){ public final void set(final Player player, final Location from, final Location to, final double yOnGround){
this.cache.setAccess(from.getWorld());
this.from.set(from, player, yOnGround); this.from.set(from, player, yOnGround);
this.cache.setAccess(from.getWorld());
this.from.setBlockCache(cache); this.from.setBlockCache(cache);
if (to != null){ if (to != null){
this.to.set(to, player, yOnGround); this.to.set(to, player, yOnGround);
this.to.setBlockCache(cache); this.to.setBlockCache(cache);
} }
useLoc.setWorld(null); // Just in case of repeated setting. // Note: using set to reset to by passing null won't work.
} }
/**
* Clear caches and remove World references and such. Also resets the world of useLoc.
*/
public final void cleanup(){ public final void cleanup(){
useLoc.setWorld(null);
from.cleanup(); from.cleanup();
to.cleanup(); to.cleanup();
cache.cleanup(); cache.cleanup();
useLoc.setWorld(null);
} }
} }

View File

@ -281,13 +281,14 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
// TODO: Add something to guess the best set back location (possibly data.guessSetBack(Location)). // TODO: Add something to guess the best set back location (possibly data.guessSetBack(Location)).
target = LocUtil.clone(loc); target = LocUtil.clone(loc);
} }
useLoc.setWorld(null);
if (sfCheck && cc.sfFallDamage && noFall.isEnabled(player)) { if (sfCheck && cc.sfFallDamage && noFall.isEnabled(player)) {
// Check if to deal damage. // Check if to deal damage.
double y = loc.getY(); double y = loc.getY();
if (data.hasSetBack()) y = Math.min(y, data.getSetBackY()); if (data.hasSetBack()) y = Math.min(y, data.getSetBackY());
noFall.checkDamage(player, data, y); noFall.checkDamage(player, data, y);
} }
// Cleanup
useLoc.setWorld(null);
// Teleport. // Teleport.
data.prepareSetBack(target); // Should be enough. | new Location(target.getWorld(), target.getX(), target.getY(), target.getZ(), target.getYaw(), target.getPitch()); data.prepareSetBack(target); // Should be enough. | new Location(target.getWorld(), target.getX(), target.getY(), target.getZ(), target.getYaw(), target.getPitch());
player.teleport(target, TeleportCause.PLUGIN);// TODO: schedule / other measures ? player.teleport(target, TeleportCause.PLUGIN);// TODO: schedule / other measures ?
@ -418,8 +419,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
// TODO: Order this to above "early return"? // TODO: Order this to above "early return"?
// Set up data / caching. // Set up data / caching.
final MoveInfo moveInfo; final MoveInfo moveInfo;
if (parkedInfo.isEmpty()) moveInfo = new MoveInfo(mcAccess); if (parkedInfo.isEmpty()) {
else moveInfo = parkedInfo.remove(parkedInfo.size() - 1); moveInfo = new MoveInfo(mcAccess);
}
else {
moveInfo = parkedInfo.remove(parkedInfo.size() - 1);
}
final MovingConfig cc = MovingConfig.getConfig(player); final MovingConfig cc = MovingConfig.getConfig(player);
moveInfo.set(player, from, to, cc.yOnGround); moveInfo.set(player, from, to, cc.yOnGround);
// TODO: Data resetting above ? // TODO: Data resetting above ?
@ -1123,8 +1128,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
boolean allowReset = true; boolean allowReset = true;
if (!data.noFallSkipAirCheck) { if (!data.noFallSkipAirCheck) {
final MoveInfo moveInfo; final MoveInfo moveInfo;
if (parkedInfo.isEmpty()) moveInfo = new MoveInfo(mcAccess); if (parkedInfo.isEmpty()) {
else moveInfo = parkedInfo.remove(parkedInfo.size() - 1); moveInfo = new MoveInfo(mcAccess);
}
else {
moveInfo = parkedInfo.remove(parkedInfo.size() - 1);
}
moveInfo.set(player, loc, null, cc.noFallyOnGround); moveInfo.set(player, loc, null, cc.noFallyOnGround);
// NOTE: No isIllegal check here. // NOTE: No isIllegal check here.
final PlayerLocation pLoc = moveInfo.from; final PlayerLocation pLoc = moveInfo.from;
@ -1150,7 +1159,6 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
moveInfo.cleanup(); moveInfo.cleanup();
parkedInfo.add(moveInfo); parkedInfo.add(moveInfo);
} }
useLoc.setWorld(null);
final float fallDistance = player.getFallDistance(); final float fallDistance = player.getFallDistance();
final double damage = BridgeHealth.getDamage(event); final double damage = BridgeHealth.getDamage(event);
final float yDiff = (float) (data.noFallMaxY - loc.getY()); final float yDiff = (float) (data.noFallMaxY - loc.getY());
@ -1183,6 +1191,8 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
} }
} }
// Entity fall-distance should be reset elsewhere. // Entity fall-distance should be reset elsewhere.
// Cleanup.
useLoc.setWorld(null);
} }
/** /**
@ -1468,8 +1478,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
return; return;
} }
final MoveInfo info; final MoveInfo info;
if (parkedInfo.isEmpty()) info = new MoveInfo(mcAccess); if (parkedInfo.isEmpty()) {
else info = parkedInfo.remove(parkedInfo.size() - 1); info = new MoveInfo(mcAccess);
}
else {
info = parkedInfo.remove(parkedInfo.size() - 1);
}
for (final String playerName : hoverTicks) { for (final String playerName : hoverTicks) {
// TODO: put players into the set (+- one tick would not matter ?) // TODO: put players into the set (+- one tick would not matter ?)
// TODO: might add an online flag to data ! // TODO: might add an online flag to data !
@ -1546,6 +1560,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
// Check if player is on ground. // Check if player is on ground.
final Location loc = player.getLocation(useLoc); // useLoc.setWorld(null) is done in onTick. final Location loc = player.getLocation(useLoc); // useLoc.setWorld(null) is done in onTick.
info.set(player, loc, null, cc.yOnGround); info.set(player, loc, null, cc.yOnGround);
// (Could use useLoc of MoveInfo here. Note orderm though.)
final boolean res; final boolean res;
// TODO: Collect flags, more margin ? // TODO: Collect flags, more margin ?
final int loaded = info.from.ensureChunksLoaded(); final int loaded = info.from.ensureChunksLoaded();

View File

@ -136,7 +136,7 @@ public class Passable extends Check {
System.out.println(player.getName() + " Using set-back location for passable."); System.out.println(player.getName() + " Using set-back location for passable.");
} }
} else if (cc.debug) { } else if (cc.debug) {
System.out.println(player.getName() + " Ignorng set-back for passable."); System.out.println(player.getName() + " Ignoring set-back for passable.");
} }
} }
@ -155,7 +155,8 @@ public class Passable extends Check {
// TODO: Consider another set back position for this, also keeping track of players moving around in blocks. // TODO: Consider another set back position for this, also keeping track of players moving around in blocks.
final Location newTo; final Location newTo;
if (loc != null) { if (loc != null) {
newTo = loc; // Ensure the given location is cloned.
newTo = LocUtil.clone(loc);
} else { } else {
newTo = from.getLocation(); newTo = from.getLocation();
if (cc.debug) { if (cc.debug) {