Merge pull request #190 from simoleo89/fix/catalog-page-mutation-guards

fix(catalog): harden admin mutations and voucher claims
This commit is contained in:
DuckieTM
2026-06-15 07:22:47 +02:00
committed by GitHub
13 changed files with 461 additions and 70 deletions
@@ -0,0 +1,50 @@
package com.eu.habbo.habbohotel.catalog;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
class VoucherClaimContractTest {
private static String voucherSource() throws Exception {
return Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/catalog/Voucher.java"));
}
private static String catalogManagerSource() throws Exception {
return Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/catalog/CatalogManager.java"));
}
@Test
void voucherClaimIsSynchronizedAndPersistsBeforeRewardEligibility() throws Exception {
String source = voucherSource();
assertTrue(source.contains("public synchronized ClaimResult claimForUser(int userId)"),
"voucher claim should check limits and persist history under a per-voucher lock");
assertTrue(source.contains("private boolean insertHistoryEntry"),
"history insert should report database failure to the caller");
int insertCall = source.indexOf("insertHistoryEntry(userId, timestamp)");
int memoryAppend = source.indexOf("this.history.add(new VoucherHistoryEntry(this.id, userId, timestamp))");
assertTrue(insertCall > -1, "claimForUser must persist the history row");
assertTrue(memoryAppend > insertCall,
"in-memory history must only be updated after the database insert succeeds");
}
@Test
void catalogRewardsOnlyAfterVoucherClaimSucceeds() throws Exception {
String source = catalogManagerSource();
int claim = source.indexOf("Voucher.ClaimResult claimResult = voucher.claimForUser");
int claimedGuard = source.indexOf("case CLAIMED", claim);
int pointsGrant = source.indexOf("client.getHabbo().givePoints", claim);
int creditsGrant = source.indexOf("client.getHabbo().giveCredits", claim);
assertTrue(claim > -1, "CatalogManager must claim the voucher before applying rewards");
assertTrue(claimedGuard > claim, "voucher rewards should only continue for a CLAIMED result");
assertTrue(pointsGrant > claimedGuard, "points must be granted only after CLAIMED");
assertTrue(creditsGrant > claimedGuard, "credits must be granted only after CLAIMED");
}
}
@@ -0,0 +1,43 @@
package com.eu.habbo.messages.incoming.catalog.catalogadmin;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
class CatalogAdminOfferMutationContractTest {
private static final Path CREATE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminCreateOfferEvent.java");
private static final Path SAVE_SOURCE = Path.of(
"src/main/java/com/eu/habbo/messages/incoming/catalog/catalogadmin/CatalogAdminSaveOfferEvent.java");
@Test
void createAndSaveValidatePayloadAndTargetPageBeforeWriting() throws IOException {
String create = Files.readString(CREATE_SOURCE);
String save = Files.readString(SAVE_SOURCE);
assertTrue(create.contains("CatalogAdminOfferPayload.validate("));
assertTrue(save.contains("CatalogAdminOfferPayload.validate("));
assertTrue(create.contains("getCatalogPage(payload.pageId, payload.pageType) == null"));
assertTrue(save.contains("getCatalogPage(payload.pageId, payload.pageType) == null"));
int createValidation = create.indexOf("CatalogAdminOfferPayload.validate(");
int createInsert = create.indexOf("INSERT INTO catalog_items");
int saveValidation = save.indexOf("CatalogAdminOfferPayload.validate(");
int saveUpdate = save.indexOf("UPDATE catalog_items");
assertTrue(createValidation < createInsert, "create offer should validate before insert SQL is prepared");
assertTrue(saveValidation < saveUpdate, "save offer should validate before update SQL is prepared");
}
@Test
void saveOfferReportsMissingRowsInsteadOfAlwaysSucceeding() throws IOException {
String save = Files.readString(SAVE_SOURCE);
assertTrue(save.contains("statement.executeUpdate() == 0"));
assertTrue(save.contains("Offer not found: "));
}
}
@@ -0,0 +1,41 @@
package com.eu.habbo.messages.incoming.catalog.catalogadmin;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import com.eu.habbo.habbohotel.catalog.CatalogPageType;
import org.junit.jupiter.api.Test;
class CatalogAdminOfferPayloadTest {
@Test
void acceptsAndNormalizesValidOfferPayload() {
CatalogAdminOfferPayload payload = CatalogAdminOfferPayload.validate(
42, "1, 2,3", "Rare Chair", 100, 5, 0, 1, 0,
"extra", true, 0, 0, 10, CatalogPageType.NORMAL);
assertNotNull(payload);
assertEquals("1,2,3", payload.itemIds);
assertEquals("Rare Chair", payload.catalogName);
}
@Test
void rejectsInvalidItemIdsAndNegativeEconomyValues() {
assertNull(CatalogAdminOfferPayload.validate(42, "1,abc", "Name", 0, 0, 0, 1, 0,
"", false, 0, 0, 0, CatalogPageType.NORMAL));
assertNull(CatalogAdminOfferPayload.validate(42, "1", "Name", -1, 0, 0, 1, 0,
"", false, 0, 0, 0, CatalogPageType.NORMAL));
assertNull(CatalogAdminOfferPayload.validate(42, "1", "Name", 0, 0, 0, 0, 0,
"", false, 0, 0, 0, CatalogPageType.NORMAL));
}
@Test
void builderOffersStillRequireSafeCommonFields() {
assertNotNull(CatalogAdminOfferPayload.validate(42, "", "BC Offer", -1, -1, -1, -1, -1,
"", false, -1, -1, 0, CatalogPageType.BUILDER));
assertNull(CatalogAdminOfferPayload.validate(0, "1", "BC Offer", 0, 0, 0, 1, 0,
"", false, 0, 0, 0, CatalogPageType.BUILDER));
assertNull(CatalogAdminOfferPayload.validate(42, "1", "", 0, 0, 0, 1, 0,
"", false, 0, 0, 0, CatalogPageType.BUILDER));
}
}
@@ -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"));
}
}