Skip to content

Commit dcbc5b2

Browse files
Fix crash issue when trying to parse bad skin JSON (#3712)
* Fix crash issue when trying to parse bad skin JSON Bad JSON sometimes occur with NPC players on servers * fixing indent to pass standardjs test * added test * changed double quotes into single quotes * replaced custom parsing with mojangson * linting * fix * lint * linting * linting * test * deleted non-working AI generated test * tested and fixed * Add tests for bad skin JSON parsing and add mojangson dependency Add two tests verifying that extractSkinInformation correctly handles both standard JSON and mojangson-formatted skin texture data without crashing. Also add mojangson as a direct dependency since it is used in lib/plugins/entities.js. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a516d1c commit dcbc5b2

3 files changed

Lines changed: 178 additions & 1 deletion

File tree

lib/plugins/entities.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const { Vec3 } = require('vec3')
22
const conv = require('../conversions')
3+
const mojangson = require('mojangson')
34
// These values are only accurate for versions 1.14 and above (crouch hitbox changes)
45
// Todo: hitbox sizes for sleeping, swimming/crawling, and flying with elytra
56
const PLAYER_HEIGHT = 1.8
@@ -953,7 +954,12 @@ function extractSkinInformation (properties) {
953954
return undefined
954955
}
955956

956-
const skinTexture = JSON.parse(Buffer.from(props.textures.value, 'base64').toString('utf8'))
957+
let skinTexture
958+
try { // Handles mojangson-style player data
959+
skinTexture = JSON.parse(Buffer.from(props.textures.value, 'base64'))
960+
} catch (e) {
961+
skinTexture = mojangson.simplify(mojangson.parse(Buffer.from(props.textures.value, 'base64').toString('utf-8')))
962+
}
957963

958964
const skinTextureUrl = skinTexture?.textures?.SKIN?.url ?? undefined
959965
const skinTextureModel = skinTexture?.textures?.SKIN?.metadata?.model ?? undefined

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"dependencies": {
2424
"minecraft-data": "^3.108.0",
2525
"minecraft-protocol": "^1.66.0",
26+
"mojangson": "^2.0.4",
2627
"prismarine-biome": "^1.1.1",
2728
"prismarine-block": "^1.22.0",
2829
"prismarine-chat": "^1.7.1",

test/internalTest.js

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,176 @@ for (const supportedVersion of mineflayer.testedVersions) {
709709
})
710710
})
711711

712+
it('does not crash when skin texture is mojangson format', (done) => {
713+
// Mojangson-style texture data (not valid JSON, but valid mojangson)
714+
const mojangsonStr = '{textures:{SKIN:{url:"http://textures.minecraft.net/texture/abc123",metadata:{model:"slim"}}}}'
715+
const mojangsonBase64 = Buffer.from(mojangsonStr).toString('base64')
716+
717+
server.on('playerJoin', (client) => {
718+
bot.on('entitySpawn', (entity) => {
719+
const player = bot.players[entity.username]
720+
assert.ok(player, 'player should exist')
721+
assert.ok(player.skinData, 'skinData should be parsed from mojangson')
722+
assert.strictEqual(player.skinData.url, 'http://textures.minecraft.net/texture/abc123')
723+
assert.strictEqual(player.skinData.model, 'slim')
724+
done()
725+
})
726+
727+
if (registry.supportFeature('playerInfoActionIsBitfield')) {
728+
client.write('player_info', {
729+
action: { add_player: true },
730+
data: [{
731+
uuid: '1-2-3-4',
732+
player: {
733+
name: 'bot5',
734+
properties: [{
735+
name: 'textures',
736+
value: mojangsonBase64,
737+
signature: ''
738+
}]
739+
},
740+
gamemode: 0,
741+
latency: 0
742+
}]
743+
})
744+
} else {
745+
client.write('player_info', {
746+
action: 'add_player',
747+
data: [{
748+
uuid: '1-2-3-4',
749+
name: 'bot5',
750+
properties: [{
751+
name: 'textures',
752+
value: mojangsonBase64,
753+
signature: ''
754+
}],
755+
gamemode: 0,
756+
ping: 0
757+
}]
758+
})
759+
}
760+
761+
if (bot.registry.supportFeature('unifiedPlayerAndEntitySpawnPacket')) {
762+
client.write('spawn_entity', {
763+
entityId: 56,
764+
objectUUID: '1-2-3-4',
765+
type: bot.registry.entitiesByName.player.internalId,
766+
x: 1,
767+
y: 2,
768+
z: 3,
769+
pitch: 0,
770+
yaw: 0,
771+
headPitch: 0,
772+
objectData: 1,
773+
velocity: { x: 0, y: 0, z: 0 },
774+
velocityX: 0,
775+
velocityY: 0,
776+
velocityZ: 0
777+
})
778+
} else {
779+
client.write('named_entity_spawn', {
780+
entityId: 56,
781+
playerUUID: '1-2-3-4',
782+
x: 1,
783+
y: 2,
784+
z: 3,
785+
yaw: 0,
786+
pitch: 0,
787+
currentItem: -1,
788+
metadata: []
789+
})
790+
}
791+
})
792+
})
793+
794+
it('does not crash when skin texture is valid JSON', (done) => {
795+
const validJson = JSON.stringify({
796+
textures: {
797+
SKIN: {
798+
url: 'http://textures.minecraft.net/texture/def456',
799+
metadata: { model: 'default' }
800+
}
801+
}
802+
})
803+
const jsonBase64 = Buffer.from(validJson).toString('base64')
804+
805+
server.on('playerJoin', (client) => {
806+
bot.on('entitySpawn', (entity) => {
807+
const player = bot.players[entity.username]
808+
assert.ok(player, 'player should exist')
809+
assert.ok(player.skinData, 'skinData should be parsed from JSON')
810+
assert.strictEqual(player.skinData.url, 'http://textures.minecraft.net/texture/def456')
811+
assert.strictEqual(player.skinData.model, 'default')
812+
done()
813+
})
814+
815+
if (registry.supportFeature('playerInfoActionIsBitfield')) {
816+
client.write('player_info', {
817+
action: { add_player: true },
818+
data: [{
819+
uuid: '1-2-3-4',
820+
player: {
821+
name: 'bot6',
822+
properties: [{
823+
name: 'textures',
824+
value: jsonBase64,
825+
signature: ''
826+
}]
827+
},
828+
gamemode: 0,
829+
latency: 0
830+
}]
831+
})
832+
} else {
833+
client.write('player_info', {
834+
action: 'add_player',
835+
data: [{
836+
uuid: '1-2-3-4',
837+
name: 'bot6',
838+
properties: [{
839+
name: 'textures',
840+
value: jsonBase64,
841+
signature: ''
842+
}],
843+
gamemode: 0,
844+
ping: 0
845+
}]
846+
})
847+
}
848+
849+
if (bot.registry.supportFeature('unifiedPlayerAndEntitySpawnPacket')) {
850+
client.write('spawn_entity', {
851+
entityId: 57,
852+
objectUUID: '1-2-3-4',
853+
type: bot.registry.entitiesByName.player.internalId,
854+
x: 1,
855+
y: 2,
856+
z: 3,
857+
pitch: 0,
858+
yaw: 0,
859+
headPitch: 0,
860+
objectData: 1,
861+
velocity: { x: 0, y: 0, z: 0 },
862+
velocityX: 0,
863+
velocityY: 0,
864+
velocityZ: 0
865+
})
866+
} else {
867+
client.write('named_entity_spawn', {
868+
entityId: 57,
869+
playerUUID: '1-2-3-4',
870+
x: 1,
871+
y: 2,
872+
z: 3,
873+
yaw: 0,
874+
pitch: 0,
875+
currentItem: -1,
876+
metadata: []
877+
})
878+
}
879+
})
880+
})
881+
712882
it('sets players[player].entity to null upon despawn', (done) => {
713883
let serverClient = null
714884
bot.once('entitySpawn', (entity) => {

0 commit comments

Comments
 (0)