Skip to content

Commit d0d05f9

Browse files
NotRykenNotRyken
committed
Improve serverside validation
1 parent 0a0c52e commit d0d05f9

File tree

7 files changed

+183
-97
lines changed

7 files changed

+183
-97
lines changed

‎common/src/main/java/dev/terminalmc/clientsort/config/ServerConfig.java‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,26 @@ public static Options serverOptions() {
5353

5454
public static class Options {
5555

56-
public static final boolean validateOperationResultsDefault = true;
57-
public boolean validateOperationResults = validateOperationResultsDefault;
56+
// Removed in v2.0.0-beta.20, retained as a reference for legacy upgrade
57+
public @Nullable Boolean validateOperationResults;
58+
59+
public static final boolean validationActiveSingleplayerDefault = false;
60+
public boolean validationActiveSingleplayer = validationActiveSingleplayerDefault;
61+
62+
public static final boolean validationActiveServerDefault = true;
63+
public boolean validationActiveServer = validationActiveServerDefault;
64+
65+
public static final boolean validateItemTypeDefault = true;
66+
public boolean validateItemType = validateItemTypeDefault;
67+
68+
public static final boolean validateStackSizeDefault = true;
69+
public boolean validateStackSize = validateStackSizeDefault;
70+
71+
public static final int validateStackSizeThresholdDefault = 32;
72+
public int validateStackSizeThreshold = validateStackSizeThresholdDefault;
73+
74+
public static final boolean alwaysLogUnexpectedResultsDefault = true;
75+
public boolean alwaysLogUnexpectedResults = alwaysLogUnexpectedResultsDefault;
5876

5977
public static final Supplier<List<ServerClassPolicy>> classPoliciesDefaultList =
6078
() -> List.of(
@@ -118,8 +136,6 @@ public static class Options {
118136

119137
// Validation
120138

121-
// Validation
122-
123139
@FunctionalInterface
124140
public interface Validator<T> {
125141

@@ -133,6 +149,16 @@ private void validate() {
133149
options.classPolicies = Options.classPoliciesValidator.validate(options.classPolicies);
134150
}
135151

152+
/**
153+
* Updates legacy config fields.
154+
*/
155+
private void upgradeLegacy() {
156+
if (options.validateOperationResults != null) {
157+
options.validateOperationResults = null;
158+
options.classPolicies = Options.classPoliciesDefault.get();
159+
}
160+
}
161+
136162
// Instance management
137163

138164
private static ServerConfig instance = null;
@@ -175,6 +201,8 @@ public static ServerConfig resetAndSave() {
175201
if (serverConfig == null) {
176202
backup();
177203
ClientSort.LOG.warn("Resetting config");
204+
} else {
205+
serverConfig.upgradeLegacy();
178206
}
179207
}
180208
return serverConfig != null ? serverConfig : new ServerConfig();

‎common/src/main/java/dev/terminalmc/clientsort/network/handler/CollectHandler.java‎

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import dev.terminalmc.clientsort.ClientSort;
2020
import dev.terminalmc.clientsort.config.ServerClassPolicy;
2121
import dev.terminalmc.clientsort.exception.PayloadHandlerException;
22-
import dev.terminalmc.clientsort.exception.PayloadHandlerException.InconsistentStateException;
2322
import dev.terminalmc.clientsort.exception.PayloadHandlerException.UnsupportedOpException;
2423
import dev.terminalmc.clientsort.network.handler.validate.PolicyManager;
2524
import dev.terminalmc.clientsort.network.payload.CollectPayload;
@@ -32,7 +31,6 @@
3231
import net.minecraft.world.item.ItemStack;
3332

3433
import static dev.terminalmc.clientsort.ClientSort.getObj;
35-
import static dev.terminalmc.clientsort.config.ServerConfig.serverOptions;
3634
import static dev.terminalmc.clientsort.network.handler.validate.SchemaValidator.validateSlotArray;
3735

3836
/**
@@ -55,14 +53,14 @@ public static void handle(
5553
payload.containerId(),
5654
(menu) -> checkPolicy(player, menu, payload.slotIds()),
5755
(menu) -> validateSlotArray(player, menu, payload.slotIds()),
58-
(menu) -> collect(menu, payload.slotIds()),
56+
(menu) -> collect(server, menu, payload.slotIds()),
5957
CollectPayload.TYPE,
6058
CollectResultPayload.TYPE,
6159
(result, message) -> new CollectResultPayload(result.code, message)
6260
));
6361
}
6462

65-
private static void collect(AbstractContainerMenu menu, int[] slotIds)
63+
private static void collect(MinecraftServer server, AbstractContainerMenu menu, int[] slotIds)
6664
throws PayloadHandlerException {
6765
// Work backwards from the end, looking for a partial stack
6866
for (int i = slotIds.length - 1; i >= 0; i--) {
@@ -95,26 +93,24 @@ private static void collect(AbstractContainerMenu menu, int[] slotIds)
9593
// stack as possible
9694
dstSlot.safeInsert(srcStack);
9795

98-
if (serverOptions().validateOperationResults) {
99-
// Check that the operation succeeded
100-
ItemStack expected = srcStackCopy.copyWithCount(Math.min(
101-
srcStackCopy.getCount() + dstStackCopy.getCount(),
102-
dstSlot.getMaxStackSize(srcStackCopy)
103-
));
104-
if (notEqual(dstSlot.getItem(), expected)) {
105-
String message = String.format(
106-
"Collect operation failed to safe-insert from slot %d with item '%s' to slot %d with item '%s': Expected '%s' in destination after set, got '%s'!",
96+
// Check that the operation succeeded
97+
ItemStack expected = srcStackCopy.copyWithCount(Math.min(
98+
srcStackCopy.getCount() + dstStackCopy.getCount(),
99+
dstSlot.getMaxStackSize(srcStackCopy)
100+
));
101+
validate(
102+
server,
103+
expected,
104+
dstSlot.getItem(),
105+
() -> String.format(
106+
"Collect operation failed to safe-insert from slot %d with item '%s' to slot %d with item '%s'",
107107
srcSlotId,
108108
srcStackCopy,
109109
dstSlotId,
110-
dstStackCopy,
111-
expected,
112-
dstSlot.getItem()
113-
);
114-
setPolicy(menu, slotIds, message);
115-
throw new InconsistentStateException(message);
116-
}
117-
}
110+
dstStackCopy
111+
),
112+
(msg) -> setPolicy(menu, slotIds, msg)
113+
);
118114

119115
// If no items remain in the source stack, stop looking
120116
if (srcStack.isEmpty())

‎common/src/main/java/dev/terminalmc/clientsort/network/handler/PayloadHandler.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import dev.terminalmc.clientsort.ClientSort;
2020
import dev.terminalmc.clientsort.exception.PayloadHandlerException;
21+
import dev.terminalmc.clientsort.exception.PayloadHandlerException.InconsistentStateException;
2122
import dev.terminalmc.clientsort.exception.PayloadHandlerException.InvalidDataException;
2223
import dev.terminalmc.clientsort.network.handler.validate.PayloadResult;
2324
import dev.terminalmc.clientsort.platform.Services;
@@ -30,6 +31,10 @@
3031
import org.jetbrains.annotations.Nullable;
3132

3233
import java.util.function.BiFunction;
34+
import java.util.function.Consumer;
35+
import java.util.function.Supplier;
36+
37+
import static dev.terminalmc.clientsort.config.ServerConfig.serverOptions;
3338

3439
/**
3540
* Provides common methods to custom payload handlers.
@@ -100,6 +105,63 @@ public static void processPayload(
100105
}
101106
}
102107

108+
public static void validate(
109+
MinecraftServer server,
110+
ItemStack expected,
111+
ItemStack actual,
112+
Supplier<String> message,
113+
Consumer<String> policySetter
114+
) throws InconsistentStateException {
115+
boolean invalid = false;
116+
boolean log = false;
117+
boolean error = false;
118+
119+
if (serverOptions().alwaysLogUnexpectedResults) {
120+
log = true;
121+
}
122+
if (server.isDedicatedServer()) {
123+
if (serverOptions().validationActiveServer) {
124+
log = true;
125+
error = true;
126+
}
127+
} else {
128+
if (serverOptions().validationActiveSingleplayer) {
129+
log = true;
130+
error = true;
131+
}
132+
}
133+
134+
if (log || serverOptions().validateItemType) {
135+
boolean mismatchedTypes = notEqual(expected, actual);
136+
if (mismatchedTypes)
137+
invalid = true;
138+
}
139+
140+
if (log || serverOptions().validateStackSize) {
141+
int sizeDifference = expected.getCount() > actual.getCount()
142+
? expected.getCount() - actual.getCount()
143+
: actual.getCount() - expected.getCount();
144+
if (sizeDifference > 0 && sizeDifference >= serverOptions().validateStackSizeThreshold)
145+
invalid = true;
146+
}
147+
148+
if (invalid && log) {
149+
String msg = String.format(
150+
"%s: Expected '%s' in destination after set, got '%s'!",
151+
message.get(),
152+
expected,
153+
actual
154+
);
155+
if (!error) {
156+
ClientSort.LOG.warn(msg);
157+
} else {
158+
ClientSort.LOG.error(msg);
159+
policySetter.accept(msg);
160+
throw new InconsistentStateException(msg);
161+
}
162+
}
163+
}
164+
103165
/**
104166
* @return the menu belonging to the player and matching the container ID.
105167
* @throws InvalidDataException if no matching menu was found, or one was found but is not valid

‎common/src/main/java/dev/terminalmc/clientsort/network/handler/SortHandler.java‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import java.util.TreeMap;
3838

3939
import static dev.terminalmc.clientsort.ClientSort.getObj;
40-
import static dev.terminalmc.clientsort.config.ServerConfig.serverOptions;
4140
import static dev.terminalmc.clientsort.network.handler.validate.SchemaValidator.validateSlotMapping;
4241

4342
/**
@@ -60,14 +59,14 @@ public static void handle(
6059
payload.containerId(),
6160
(menu) -> checkPolicy(player, menu, payload.slotMapping()),
6261
(menu) -> validateSlotMapping(player, menu, payload.slotMapping()),
63-
(menu) -> sort(menu, payload.slotMapping()),
62+
(menu) -> sort(server, menu, payload.slotMapping()),
6463
SortPayload.TYPE,
6564
SortResultPayload.TYPE,
6665
(result, message) -> new SortResultPayload(result.code, message)
6766
));
6867
}
6968

70-
private static void sort(AbstractContainerMenu menu, int[] slotMapping)
69+
private static void sort(MinecraftServer server, AbstractContainerMenu menu, int[] slotMapping)
7170
throws PayloadHandlerException {
7271
// Build reference map
7372
Map<Integer, ItemStack> stacks = new TreeMap<>();
@@ -82,26 +81,30 @@ private static void sort(AbstractContainerMenu menu, int[] slotMapping)
8281
// Perform the mapping set
8382
dstSlot.setByPlayer(stacks.get(srcSlotId));
8483

85-
if (serverOptions().validateOperationResults) {
86-
// Check that the operation succeeded
87-
if (notEqual(dstSlot.getItem(), stacks.get(srcSlotId))) {
88-
// Operation failed; attempt to revert all changes
89-
for (int j = 0; j <= i; j += 2) {
90-
srcSlotId = slotMapping[j];
91-
menu.slots.get(srcSlotId).set(stacks.get(srcSlotId));
92-
dstSlotId = slotMapping[j + 1];
93-
menu.slots.get(dstSlotId).set(stacks.get(dstSlotId));
94-
}
95-
String message = String.format(
96-
"Sort operation failed at slot mapping %d->%d: Expected '%s' in destination after set, got '%s'!",
97-
srcSlotId,
98-
dstSlotId,
99-
stacks.get(srcSlotId),
100-
dstSlot.getItem()
101-
);
102-
setPolicy(menu, slotMapping, message);
103-
throw new InconsistentStateException(message);
84+
// Check that the operation succeeded
85+
try {
86+
int finalSrcSlotId = srcSlotId;
87+
int finalDstSlotId = dstSlotId;
88+
validate(
89+
server,
90+
stacks.get(srcSlotId),
91+
dstSlot.getItem(),
92+
() -> String.format(
93+
"Sort operation failed at slot mapping %d->%d",
94+
finalSrcSlotId,
95+
finalDstSlotId
96+
),
97+
(msg) -> setPolicy(menu, slotMapping, msg)
98+
);
99+
} catch (InconsistentStateException e) {
100+
// Attempt to revert changes
101+
for (int j = 0; j <= i; j += 2) {
102+
srcSlotId = slotMapping[j];
103+
menu.slots.get(srcSlotId).set(stacks.get(srcSlotId));
104+
dstSlotId = slotMapping[j + 1];
105+
menu.slots.get(dstSlotId).set(stacks.get(dstSlotId));
104106
}
107+
throw e;
105108
}
106109
}
107110
}

‎common/src/main/java/dev/terminalmc/clientsort/network/handler/StackFillHandler.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import dev.terminalmc.clientsort.ClientSort;
2020
import dev.terminalmc.clientsort.config.ServerClassPolicy;
2121
import dev.terminalmc.clientsort.exception.PayloadHandlerException;
22-
import dev.terminalmc.clientsort.exception.PayloadHandlerException.InconsistentStateException;
2322
import dev.terminalmc.clientsort.exception.PayloadHandlerException.UnsupportedOpException;
2423
import dev.terminalmc.clientsort.network.handler.validate.PolicyManager;
2524
import dev.terminalmc.clientsort.network.payload.StackFillPayload;
@@ -32,7 +31,6 @@
3231
import net.minecraft.world.item.ItemStack;
3332

3433
import static dev.terminalmc.clientsort.ClientSort.getObj;
35-
import static dev.terminalmc.clientsort.config.ServerConfig.serverOptions;
3634
import static dev.terminalmc.clientsort.network.handler.validate.SchemaValidator.validateSlotArray;
3735

3836
/**
@@ -58,15 +56,19 @@ public static void handle(
5856
validateSlotArray(player, menu, payload.srcSlotIds());
5957
validateSlotArray(player, menu, payload.dstSlotIds());
6058
},
61-
(menu) -> fillStacks(menu, payload.srcSlotIds(), payload.dstSlotIds()),
59+
(menu) -> fillStacks(server, menu, payload.srcSlotIds(), payload.dstSlotIds()),
6260
StackFillPayload.TYPE,
6361
StackFillResultPayload.TYPE,
6462
(result, message) -> new StackFillResultPayload(result.code, message)
6563
));
6664
}
6765

68-
private static void fillStacks(AbstractContainerMenu menu, int[] srcSlotIds, int[] dstSlotIds)
69-
throws PayloadHandlerException {
66+
private static void fillStacks(
67+
MinecraftServer server,
68+
AbstractContainerMenu menu,
69+
int[] srcSlotIds,
70+
int[] dstSlotIds
71+
) throws PayloadHandlerException {
7072
// Work backwards from the end of the source array, looking for a
7173
// nonempty stack
7274
for (int i = srcSlotIds.length - 1; i >= 0; i--) {
@@ -103,22 +105,20 @@ private static void fillStacks(AbstractContainerMenu menu, int[] srcSlotIds, int
103105
// Place as much of the source stack as possible
104106
dstSlot.safeInsert(srcStack);
105107

106-
if (serverOptions().validateOperationResults) {
107-
// Check that the operation succeeded
108-
if (notEqual(dstSlot.getItem(), expected)) {
109-
String message = String.format(
110-
"Stack Fill operation failed to safe-insert from slot %d with item '%s' to slot %d with item '%s': Expected '%s' in destination after set, got '%s'!",
108+
// Check that the operation succeeded
109+
validate(
110+
server,
111+
expected,
112+
dstSlot.getItem(),
113+
() -> String.format(
114+
"Stack Fill operation failed to safe-insert from slot %d with item '%s' to slot %d with item '%s'",
111115
srcSlotId,
112116
srcStackCopy,
113117
dstSlotId,
114-
dstStackCopy,
115-
expected,
116-
dstSlot.getItem()
117-
);
118-
setPolicy(menu, dstSlotIds, message);
119-
throw new InconsistentStateException(message);
120-
}
121-
}
118+
dstStackCopy
119+
),
120+
(msg) -> setPolicy(menu, dstSlotIds, msg)
121+
);
122122

123123
// If no items remain in the source stack, stop looking
124124
if (srcStack.isEmpty())

0 commit comments

Comments
 (0)