Skip to content

Commit c526a28

Browse files
committed
fix(blacklist): Wait for ConnectRequest
Somehow mobile clients basically skipped sending ConnectRequest if we didn't wait for it first
1 parent e59c72f commit c526a28

3 files changed

Lines changed: 247 additions & 59 deletions

File tree

app/dimensions/blacklistcheckclient.ts

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ import RawPacket from './packets/rawpacket.js';
33
import { getPacketsFromBuffer, BuffersPackets } from './utils.js';
44
import { getProperIP } from './utils.js';
55
import Blacklist from './blacklist.js';
6-
import { Parser, PlayerSlotSetPacket } from 'terraria-packet';
6+
import NetworkText from '@popstarfreas/packetfactory/networktext';
7+
import { Parser, PlayerSlotSetPacket, StatusPacket } from 'terraria-packet';
78
import { BlacklistCheckContext, BlacklistCheckState, RawSocketWriteContext, RawSocketWriteReason } from './extension/index.js';
89
import ErrorHelper from './errorhelper.js';
910

1011
enum ClientState {
11-
StartOfConnection,
12+
WaitingForConnectRequest,
1213
AssignedClientId,
1314
SentPlayerInfo,
1415
SentUuid,
@@ -28,7 +29,7 @@ interface BlacklistCheckCallbackArgs {
2829
}
2930

3031
class BlacklistCheckClient {
31-
private state: ClientState = ClientState.StartOfConnection;
32+
private state: ClientState = ClientState.WaitingForConnectRequest;
3233
private name: string | undefined;
3334
private bufferPacket: Buffer = Buffer.alloc(0);
3435
private packetsReceived: RawPacket[] = [];
@@ -39,7 +40,6 @@ class BlacklistCheckClient {
3940
private disposed: boolean = false;
4041

4142
constructor(private settings: BlacklistCheckClientArgs) {
42-
this.state = ClientState.AssignedClientId;
4343
}
4444

4545
setupCallbacks(args: BlacklistCheckCallbackArgs) {
@@ -58,15 +58,6 @@ class BlacklistCheckClient {
5858
args.disconnectCb();
5959
this.dispose()
6060
})
61-
const playerSlotSetPacket = PlayerSlotSetPacket.toBuffer({
62-
playerSlotId: 0,
63-
serverWantsToRunCheckBytesInClientLoopThread: true,
64-
});
65-
if (playerSlotSetPacket.TAG === "Error") {
66-
this.settings.clientArgs.logging.error(`Error creating player slot set packet: ${playerSlotSetPacket._0}`);
67-
return;
68-
}
69-
this.writeToSocket(playerSlotSetPacket._0, RawSocketWriteReason.BlacklistCheckClientSetup);
7061
}
7162

7263
handleData(data: Buffer) {
@@ -116,10 +107,26 @@ class BlacklistCheckClient {
116107
const packet = packetResult._0;
117108

118109
switch (packet.TAG) {
110+
case "ConnectRequest":
111+
if (this.state !== ClientState.WaitingForConnectRequest) {
112+
this.dispose()
113+
this.packetErrorCheckingBlacklistCb(new Error("Connect request packet received after assigning client ID"))
114+
return
115+
}
116+
const connectRequestResult = packet._0.VAL();
117+
if (connectRequestResult.TAG === "Error") {
118+
this.dispose()
119+
this.packetErrorCheckingBlacklistCb(new Error("Connect request packet could not be parsed. Error: " + connectRequestResult._0.context))
120+
return
121+
}
122+
123+
this.state = ClientState.AssignedClientId;
124+
this.sendBlacklistCheckSetupPackets();
125+
break;
119126
case "PlayerInfo":
120127
if (this.state !== ClientState.AssignedClientId) {
121128
this.dispose()
122-
this.packetErrorCheckingBlacklistCb(new Error("Client info packet received before assigning client ID"))
129+
this.packetErrorCheckingBlacklistCb(new Error("Client info packet received before connect request"))
123130
return
124131
}
125132
const playerInfoResult = packet._0.VAL();
@@ -183,6 +190,48 @@ class BlacklistCheckClient {
183190
this.runPostHandlers(rawPacket);
184191
}
185192

193+
private sendBlacklistCheckSetupPackets(): void {
194+
this.sendCheckingStatus();
195+
this.sendPlayerSlotSet();
196+
}
197+
198+
private sendCheckingStatus(): void {
199+
const statusPacket = StatusPacket.toBuffer({
200+
max: 0,
201+
text: new NetworkText(0, "Checking access..."),
202+
flags: {
203+
hideStatusTextPercent: true,
204+
statusTextHasShadows: true,
205+
runCheckBytes: false
206+
}
207+
});
208+
209+
switch (statusPacket.TAG) {
210+
case "Ok":
211+
this.writeToSocket(statusPacket._0, RawSocketWriteReason.BlacklistCheck);
212+
break;
213+
case "Error":
214+
this.settings.clientArgs.logging.error(`Error creating status packet: ${statusPacket._0}`);
215+
break;
216+
}
217+
}
218+
219+
private sendPlayerSlotSet(): void {
220+
const playerSlotSetPacket = PlayerSlotSetPacket.toBuffer({
221+
playerSlotId: 0,
222+
serverWantsToRunCheckBytesInClientLoopThread: false,
223+
});
224+
225+
switch (playerSlotSetPacket.TAG) {
226+
case "Ok":
227+
this.writeToSocket(playerSlotSetPacket._0, RawSocketWriteReason.BlacklistCheckClientSetup);
228+
break;
229+
case "Error":
230+
this.settings.clientArgs.logging.error(`Error creating player slot set packet: ${playerSlotSetPacket._0}`);
231+
break;
232+
}
233+
}
234+
186235
handleError(err: Error) {
187236
console.error("Error with client", err);
188237
}

app/dimensions/listenserver.ts

Lines changed: 1 addition & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import ErrorHelper from './errorhelper.js';
1717
import BlacklistCheckClient from './blacklistcheckclient.js';
1818
import * as winston from 'winston';
1919
import { RawSocketWriteContext, RawSocketWriteReason } from './extension/index.js';
20-
import { DisconnectPacket, StatusPacket } from 'terraria-packet';
20+
import { DisconnectPacket } from 'terraria-packet';
2121
import {
2222
DisconnectReason,
2323
DisconnectReasonCodes,
@@ -256,22 +256,6 @@ export class ListenServer {
256256
}
257257
}
258258

259-
private writeToSocketWithHooks(
260-
socket: Net.Socket,
261-
packet: Buffer,
262-
reason: RawSocketWriteContext['reason'],
263-
clientArgs?: ClientArgs
264-
): boolean {
265-
const prepared = this.prepareSocketPacketWithHooks(socket, packet, reason, clientArgs);
266-
if (prepared === null) {
267-
return false;
268-
}
269-
270-
socket.write(prepared.packetWrapper.packet);
271-
this.runRawSocketWritePostHandlers(prepared.context, prepared.packetWrapper);
272-
return true;
273-
}
274-
275259
/**
276260
* Sends the client the disconnect packet and then drops the connection
277261
*
@@ -507,7 +491,6 @@ export class ListenServer {
507491
// and then get checked before they are allowed to connect to a server
508492
if (this.options.blacklist.enabled && this.blacklist) {
509493
const configuration = this.options.blacklist;
510-
this.sendCheckingIp(clientArgs);
511494
let client = new BlacklistCheckClient({
512495
blacklist: this.blacklist,
513496
clientArgs,
@@ -618,33 +601,6 @@ export class ListenServer {
618601
}
619602
}
620603

621-
/**
622-
* Tells the client that its information is being checked
623-
*
624-
* @param client The client whose information is being checked
625-
*/
626-
private sendCheckingIp(client: ClientArgs): void {
627-
const msg = "Checking access...";
628-
let statusPacket = StatusPacket.toBuffer({
629-
max: 0,
630-
text: new NetworkText(0, msg),
631-
flags: {
632-
hideStatusTextPercent: true,
633-
statusTextHasShadows: true,
634-
runCheckBytes: false
635-
}
636-
})
637-
638-
switch (statusPacket.TAG) {
639-
case "Ok":
640-
this.writeToSocketWithHooks(client.socket, statusPacket._0, RawSocketWriteReason.BlacklistCheck, client);
641-
break;
642-
case "Error":
643-
this.logging.error(`Error creating status packet: ${statusPacket._0}`);
644-
break;
645-
}
646-
}
647-
648604
/**
649605
* Hook the socket error and pass it into the client object
650606
*
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
import { EventEmitter } from 'events';
2+
import * as Net from 'net';
3+
import * as winston from 'winston';
4+
import Blacklist from '../../dimensions/blacklist.js';
5+
import BlacklistCheckClient from '../../dimensions/blacklistcheckclient.js';
6+
import ClientArgs from '../../dimensions/clientargs.js';
7+
import RawPacket from '../../dimensions/packets/rawpacket.js';
8+
import { ClientUuidPacket, ConnectRequestPacket, Parser, PlayerInfoPacket } from 'terraria-packet';
9+
10+
describe("BlacklistCheckClient", () => {
11+
const uuid = "29be7f8f-25ae-4c10-9664-6f7c783cba32";
12+
13+
function unwrapBuffer(result: { TAG: "Ok"; _0: Buffer } | { TAG: "Error"; _0: unknown }): Buffer {
14+
if (result.TAG === "Error") {
15+
throw new Error(`Error creating packet: ${String(result._0)}`);
16+
}
17+
18+
return result._0;
19+
}
20+
21+
function makeSocket(): Net.Socket & { write: jasmine.Spy } {
22+
const socket = new EventEmitter() as Net.Socket & { write: jasmine.Spy };
23+
Object.defineProperty(socket, "remoteAddress", {
24+
value: "127.0.0.1",
25+
configurable: true
26+
});
27+
socket.write = jasmine.createSpy("write");
28+
29+
return socket;
30+
}
31+
32+
function makeClientArgs(socket: Net.Socket): ClientArgs {
33+
return {
34+
socket,
35+
globalHandlers: {
36+
extensions: {}
37+
},
38+
options: {
39+
log: {
40+
extensionError: false
41+
}
42+
},
43+
logging: winston.createLogger({ silent: true })
44+
} as unknown as ClientArgs;
45+
}
46+
47+
function makeBlacklist(isBlacklisted = false): Blacklist & { checkInformation: jasmine.Spy } {
48+
return {
49+
checkInformation: jasmine.createSpy("checkInformation").and.returnValue(Promise.resolve(isBlacklisted))
50+
} as unknown as Blacklist & { checkInformation: jasmine.Spy };
51+
}
52+
53+
function makeClient(socket = makeSocket(), blacklist = makeBlacklist()) {
54+
const client = new BlacklistCheckClient({
55+
blacklist,
56+
clientArgs: makeClientArgs(socket)
57+
});
58+
const accepted: { bufferPacket: Buffer; packetsReceived: RawPacket[] }[] = [];
59+
const callbacks = {
60+
clientAcceptedCb: (bufferPacket: Buffer, packetsReceived: RawPacket[]) => {
61+
accepted.push({ bufferPacket, packetsReceived });
62+
},
63+
clientBlacklistedCb: jasmine.createSpy("clientBlacklistedCb"),
64+
errorCheckingBlacklistCb: jasmine.createSpy("errorCheckingBlacklistCb"),
65+
packetErrorCheckingBlacklistCb: jasmine.createSpy("packetErrorCheckingBlacklistCb"),
66+
disconnectCb: jasmine.createSpy("disconnectCb")
67+
};
68+
69+
client.setupCallbacks(callbacks);
70+
71+
return { client, socket, blacklist, callbacks, accepted };
72+
}
73+
74+
function connectRequestPacket(): Buffer {
75+
return unwrapBuffer(ConnectRequestPacket.toBuffer({ version: "Terraria319" }));
76+
}
77+
78+
function playerInfoPacket(): Buffer {
79+
const color = { R: 0, G: 0, B: 0 };
80+
return unwrapBuffer(PlayerInfoPacket.toBuffer({
81+
playerId: 0,
82+
skinVariant: 1,
83+
voiceVariant: 1,
84+
voicePitchOffset: 0,
85+
hair: 1,
86+
name: "Smekku",
87+
hairDye: 0,
88+
hideVisuals: 0,
89+
hideVisuals2: 0,
90+
hideMisc: 0,
91+
hairColor: color,
92+
skinColor: color,
93+
eyeColor: color,
94+
shirtColor: color,
95+
underShirtColor: color,
96+
pantsColor: color,
97+
shoeColor: color,
98+
difficulty: "Softcore",
99+
mode: "Classic",
100+
extraAccessory: false,
101+
usingBiomeTorches: false,
102+
unlockedBiomeTorches: false,
103+
happyFunTorchTime: false,
104+
unlockedSuperCart: false,
105+
enabledSuperCart: false,
106+
usedAegisCrystal: false,
107+
usedAegisFruit: false,
108+
usedArcaneCrystal: false,
109+
usedGalaxyPearl: false,
110+
usedGummyWorm: false,
111+
usedAmbrosia: false,
112+
ateArtisanBread: false
113+
}));
114+
}
115+
116+
function clientUuidPacket(): Buffer {
117+
return unwrapBuffer(ClientUuidPacket.toBuffer({ uuid }));
118+
}
119+
120+
function parseServerPacketTag(packet: Buffer): string {
121+
const parsed = Parser.parse(packet, true);
122+
if (parsed.TAG === "Error") {
123+
throw new Error(`Error parsing packet: ${String(parsed._0)}`);
124+
}
125+
126+
return parsed._0.TAG;
127+
}
128+
129+
function parseClientPacketTag(packet: Buffer): string {
130+
const parsed = Parser.parse(packet, false);
131+
if (parsed.TAG === "Error") {
132+
throw new Error(`Error parsing packet: ${String(parsed._0)}`);
133+
}
134+
135+
return parsed._0.TAG;
136+
}
137+
138+
it("does not write setup packets before receiving ConnectRequest", () => {
139+
const { socket } = makeClient();
140+
141+
expect(socket.write).not.toHaveBeenCalled();
142+
});
143+
144+
it("writes status then player slot setup after receiving ConnectRequest", () => {
145+
const { client, socket } = makeClient();
146+
147+
client.handleData(connectRequestPacket());
148+
149+
expect(socket.write).toHaveBeenCalledTimes(2);
150+
const writes = socket.write.calls.allArgs().map(args => args[0] as Buffer);
151+
expect(parseServerPacketTag(writes[0])).toBe("Status");
152+
expect(parseServerPacketTag(writes[1])).toBe("PlayerSlotSet");
153+
});
154+
155+
it("accepts clients after ConnectRequest, PlayerInfo, and ClientUuid and replays ConnectRequest first", async () => {
156+
const { client, blacklist, accepted } = makeClient();
157+
158+
client.handleData(Buffer.concat([
159+
connectRequestPacket(),
160+
playerInfoPacket(),
161+
clientUuidPacket()
162+
]));
163+
await Promise.resolve();
164+
165+
expect(blacklist.checkInformation).toHaveBeenCalledOnceWith("Smekku", "127.0.0.1", uuid);
166+
expect(accepted.length).toBe(1);
167+
expect(accepted[0].bufferPacket.length).toBe(0);
168+
expect(accepted[0].packetsReceived.map(packet => parseClientPacketTag(packet.data))).toEqual([
169+
"ConnectRequest",
170+
"PlayerInfo",
171+
"ClientUuid"
172+
]);
173+
});
174+
175+
it("rejects PlayerInfo before ConnectRequest", () => {
176+
const { client, socket, callbacks } = makeClient();
177+
178+
client.handleData(playerInfoPacket());
179+
180+
expect(callbacks.packetErrorCheckingBlacklistCb).toHaveBeenCalledOnceWith(jasmine.any(Error));
181+
expect(socket.write).not.toHaveBeenCalled();
182+
});
183+
});

0 commit comments

Comments
 (0)