feat: skip SRV lookup for IP addresses and localhost

Performing DNS SRV lookups for hosts that are already direct IP
addresses or special-case hostnames like 'localhost' is
unnecessary.

In addition, several related test suite improvements
have been included:

- A bug in the Bedrock mock packet creation was fixed where the magic
  GUID was being written to an incorrect buffer offset.
- The DNS mocking strategy in the Java tests has been refactored for
  better accuracy by mocking the `dns.promises.Resolver` class.
- An error test for the Bedrock pinger was corrected to properly
  handle a promise rejection instead of a synchronous throw.

lib/java.js

@@ -5,7 +5,7 @@
 
 "use strict";
 
-import net from "node:net";
+import { createConnection, isIP } from "node:net";
 import { Resolver } from "node:dns/promises";
 import createDebug from "debug";
 import * as varint from "./varint.js";
@@ -147,39 +147,60 @@ export async function pingJava(host, options = {}) {
 	let targetHost = host;
 	let targetPort = fallbackPort;
 
-	try {
+	// A list of hostnames that should never have an SRV lookup.
+	const nonSrvLookableHostnames = ["localhost"];
+
+	// Check if the host is a valid IP address (v4 or v6).
+	// net.isIP() returns 0 for invalid IPs, 4 for IPv4, and 6 for IPv6.
+	const isDirectIp = isIP(host) !== 0;
+	const isNonLookableHostname = nonSrvLookableHostnames.includes(
+		host.toLowerCase()
+	);
+
+	if (isDirectIp || isNonLookableHostname) {
 		debug(
-			"attempting SRV lookup for _minecraft._tcp.%s with %dms timeout",
+			"host '%s' is a direct IP or a non-lookable hostname, skipping SRV lookup.",
-			host,
+			host
-			timeout
 		);
-		const resolver = new Resolver({ timeout, tries: 3 });
+	} else {
-		const srvRecords = await resolver.resolveSrv(`_minecraft._tcp.${host}`);
+		try {
-		if (srvRecords.length > 0) {
+			debug(
-			targetHost = srvRecords[0].name;
+				"attempting SRV lookup for _minecraft._tcp.%s with %dms timeout",
-			targetPort = srvRecords[0].port;
+				host,
-			debug("SRV lookup successful, new target: %s:%d", targetHost, targetPort);
+				timeout
-		}
+			);
-	} catch (err) {
+			const resolver = new Resolver({ timeout, tries: 3 });
-		// Common errors like ENODATA, ENOTFOUND, or a DNS timeout (ETIMEOUT) are expected
+			const srvRecords = await resolver.resolveSrv(`_minecraft._tcp.${host}`);
-		// when a server does not have an SRV record, so we ignore them and proceed.
+			if (srvRecords.length > 0) {
-		const nonFatalDnsCodes = ["ENODATA", "ENOTFOUND", "ETIMEOUT"];
+				targetHost = srvRecords[0].name;
-		if (
+				targetPort = srvRecords[0].port;
-			err instanceof Error &&
+				debug(
-			"code" in err &&
+					"SRV lookup successful, new target: %s:%d",
-			nonFatalDnsCodes.includes(err.code)
+					targetHost,
-		) {
+					targetPort
-			debug("SRV lookup for %s failed (%s), using fallback.", host, err.code);
+				);
-		} else {
+			}
-			// Re-throw anything else to fail the operation
+		} catch (err) {
-			debug("SRV lookup for %s failed unexpectedly, re-throwing.", host, err);
+			// Common errors like ENODATA, ENOTFOUND, or a DNS timeout (ETIMEOUT) are expected
-			throw err;
+			// when a server does not have an SRV record, so we ignore them and proceed.
+			const nonFatalDnsCodes = ["ENODATA", "ENOTFOUND", "ETIMEOUT"];
+			if (
+				err instanceof Error &&
+				"code" in err &&
+				nonFatalDnsCodes.includes(err.code)
+			) {
+				debug("SRV lookup for %s failed (%s), using fallback.", host, err.code);
+			} else {
+				// Re-throw anything else to fail the operation
+				debug("SRV lookup for %s failed unexpectedly, re-throwing.", host, err);
+				throw err;
+			}
 		}
 	}
 
 	return new Promise((resolve, reject) => {
 		debug("creating TCP connection to %s:%d", targetHost, targetPort);
-		const socket = net.createConnection({ host: targetHost, port: targetPort });
+		const socket = createConnection({ host: targetHost, port: targetPort });
 
 		// Prevent cleanup tasks from running more than once
 		// in case of multiple error callbacks

test/bedrock.test.js

@@ -125,8 +125,10 @@ describe("bedrock.js", () => {
 	});
 
 	describe("errors", () => {
-		it("should throw an error if host is not provided", () => {
+		it("should throw an error if host is not provided", async () => {
-			expect(() => pingBedrock(null)).toThrow("Host argument is required");
+			await expect(pingBedrock(null)).rejects.toThrow(
+				"Host argument is required"
+			);
 		});
 
 		it("should reject on socket timeout", async () => {
@@ -167,7 +169,7 @@ function createMockPongPacket(motd) {
 	const packet = Buffer.alloc(35 + motdBuffer.length);
 	packet.writeUInt8(0x1c, 0);
 	packet.writeBigInt64LE(BigInt(Date.now()), 1);
-	Buffer.from("00ffff00fefefefefdfdfdfd12345678", "hex").copy(packet, 17);
+	Buffer.from("00ffff00fefefefefdfdfdfd12345678", "hex").copy(packet, 9);
 	packet.writeUInt16BE(motdBuffer.length, 33);
 	motdBuffer.copy(packet, 35);
 	return packet;

test/java.test.js

@@ -1,18 +1,25 @@
 import net from "node:net";
-import dns from "node:dns/promises";
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { pingJava } from "../lib/java.js";
 import * as varint from "../lib/varint.js";
 
+const mockResolveSrv = vi.fn();
+
 vi.mock("node:net");
-vi.mock("node:dns/promises");
+vi.mock("node:dns/promises", () => ({
+	Resolver: vi.fn().mockImplementation(() => ({
+		resolveSrv: mockResolveSrv,
+	})),
+}));
 
 describe("pingJava", () => {
 	let mockSocket;
 
 	beforeEach(() => {
-		// Simulate no SRV record found.
+		// Reset mocks before each test.
-		dns.resolveSrv.mockResolvedValue([]);
+		mockResolveSrv.mockClear();
+		// Simulate no SRV record found by default.
+		mockResolveSrv.mockResolvedValue([]);
 
 		const mockHandlers = {};
 		mockSocket = {
@@ -44,7 +51,11 @@ describe("pingJava", () => {
 		// Allow the async SRV lookup to complete
 		await vi.runAllTicks();
 
-		expect(dns.resolveSrv).toHaveBeenCalledWith(`_minecraft._tcp.${host}`);
+		expect(net.createConnection).toHaveBeenCalledWith({
+			host: host,
+			port: options.port,
+		});
+
 		expect(net.createConnection).toHaveBeenCalledWith({
 			host,
 			port: options.port,