fix(catalog): guard page mutations

This commit is contained in:
simoleo89
2026-06-14 16:39:45 +02:00
parent 87e1ef94f7
commit c9214bac07
5 changed files with 92 additions and 20 deletions
@@ -36,7 +36,7 @@ public class CatalogAdminCreatePageEvent extends MessageHandler {
pageLayout = CatalogPageLayouts.default_3x3;
}
if (parentId != -1 && Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(parentId) == null) {
if (parentId != -1 && Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(parentId, pageType) == null) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Parent page not found: " + parentId));
return;
}
@@ -36,7 +36,10 @@ public class CatalogAdminDeletePageEvent extends MessageHandler {
try (Connection connection = Emulator.getDatabase().getDataSource().getConnection();
PreparedStatement statement = connection.prepareStatement(query)) {
statement.setInt(1, pageId);
statement.execute();
if (statement.executeUpdate() == 0) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
}
Emulator.getGameEnvironment().getCatalogManager().getCatalogPagesMap(pageType).remove(pageId);
@@ -28,12 +28,21 @@ public class CatalogAdminMovePageEvent extends MessageHandler {
CatalogPageType pageType = CatalogPageType.fromString(this.packet.readString());
String tableName = (pageType == CatalogPageType.BUILDER) ? "catalog_pages_bc" : "catalog_pages";
CatalogPage page = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(pageId, pageType);
if (page == null) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
if (newParentId == -1) {
try (Connection connection = Emulator.getDatabase().getDataSource().getConnection();
PreparedStatement statement = connection.prepareStatement(
"UPDATE " + tableName + " SET enabled = IF(enabled = '1', '0', '1') WHERE id = ?")) {
statement.setInt(1, pageId);
statement.execute();
if (statement.executeUpdate() == 0) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
}
this.client.sendResponse(new CatalogAdminResultComposer(true, "Page toggled"));
return;
@@ -44,30 +53,27 @@ public class CatalogAdminMovePageEvent extends MessageHandler {
PreparedStatement statement = connection.prepareStatement(
"UPDATE " + tableName + " SET visible = IF(visible = '1', '0', '1') WHERE id = ?")) {
statement.setInt(1, pageId);
statement.execute();
if (statement.executeUpdate() == 0) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
}
this.client.sendResponse(new CatalogAdminResultComposer(true, "Visibility toggled"));
return;
}
CatalogPage page = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(pageId, pageType);
if (page == null) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
if (newParentId == pageId) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "A page cannot be its own parent"));
return;
}
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(newParentId);
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(newParentId, pageType);
if (parent == null) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Parent page not found: " + newParentId));
return;
}
if (this.wouldCreateCycle(pageId, newParentId)) {
if (this.wouldCreateCycle(pageId, newParentId, pageType)) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Refusing to move: that would create a cycle"));
return;
}
@@ -80,18 +86,21 @@ public class CatalogAdminMovePageEvent extends MessageHandler {
statement.setInt(1, newParentId);
statement.setInt(2, newIndex);
statement.setInt(3, pageId);
statement.execute();
if (statement.executeUpdate() == 0) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
}
this.client.sendResponse(new CatalogAdminResultComposer(true, "Page moved"));
}
private boolean wouldCreateCycle(int pageId, int parentId) {
private boolean wouldCreateCycle(int pageId, int parentId, CatalogPageType pageType) {
int current = parentId;
for (int hops = 0; hops < MAX_PARENT_WALK; hops++) {
if (current == ROOT_PARENT_ID) return false;
if (current == pageId) return true;
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(current);
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(current, pageType);
if (parent == null) return false;
current = parent.getParentId();
}
@@ -74,13 +74,13 @@ public class CatalogAdminSavePageEvent extends MessageHandler {
return;
}
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(parentId);
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(parentId, pageType);
if (parent == null) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Parent page not found: " + parentId));
return;
}
if (this.wouldCreateCycle(pageId, parentId)) {
if (this.wouldCreateCycle(pageId, parentId, pageType)) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Refusing to re-parent: that would create a cycle"));
return;
}
@@ -144,18 +144,21 @@ public class CatalogAdminSavePageEvent extends MessageHandler {
statement.setInt(15, pageId);
}
statement.execute();
if (statement.executeUpdate() == 0) {
this.client.sendResponse(new CatalogAdminResultComposer(false, "Page not found: " + pageId));
return;
}
}
this.client.sendResponse(new CatalogAdminResultComposer(true, "Page saved"));
}
private boolean wouldCreateCycle(int pageId, int parentId) {
private boolean wouldCreateCycle(int pageId, int parentId, CatalogPageType pageType) {
int current = parentId;
for (int hops = 0; hops < MAX_PARENT_WALK; hops++) {
if (current == ROOT_PARENT_ID) return false;
if (current == pageId) return true;
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(current);
CatalogPage parent = Emulator.getGameEnvironment().getCatalogManager().getCatalogPage(current, pageType);
if (parent == null) return false;
current = parent.getParentId();
}
@@ -0,0 +1,57 @@
package com.eu.habbo.messages.incoming.catalog.catalogadmin;
import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.assertTrue;
class CatalogAdminPageMutationContractTest {
private static final Path CREATE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminCreatePageEvent.java");
private static final Path SAVE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminSavePageEvent.java");
private static final Path MOVE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminMovePageEvent.java");
private static final Path DELETE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminDeletePageEvent.java");
@Test
void pageParentChecksStayWithinTheSameCatalogPageType() throws IOException {
String create = Files.readString(CREATE_SOURCE);
String save = Files.readString(SAVE_SOURCE);
String move = Files.readString(MOVE_SOURCE);
assertTrue(create.contains("getCatalogPage(parentId, pageType)"));
assertTrue(save.contains("getCatalogPage(parentId, pageType)"));
assertTrue(save.contains("getCatalogPage(current, pageType)"));
assertTrue(move.contains("getCatalogPage(newParentId, pageType)"));
assertTrue(move.contains("getCatalogPage(current, pageType)"));
}
@Test
void movePageValidatesTargetBeforeTogglingVisibilityOrEnabledState() throws IOException {
String move = Files.readString(MOVE_SOURCE);
int pageLookup = move.indexOf("getCatalogPage(pageId, pageType)");
int enabledToggle = move.indexOf("SET enabled = IF");
int visibleToggle = move.indexOf("SET visible = IF");
assertTrue(pageLookup >= 0, "move page should load the page before mutating it");
assertTrue(pageLookup < enabledToggle, "enabled toggle must not run before page existence is checked");
assertTrue(pageLookup < visibleToggle, "visible toggle must not run before page existence is checked");
}
@Test
void pageMutationsReportMissingRowsInsteadOfAlwaysSucceeding() throws IOException {
String save = Files.readString(SAVE_SOURCE);
String move = Files.readString(MOVE_SOURCE);
String delete = Files.readString(DELETE_SOURCE);
assertTrue(save.contains("statement.executeUpdate() == 0"));
assertTrue(move.contains("statement.executeUpdate() == 0"));
assertTrue(delete.contains("statement.executeUpdate() == 0"));
}
}