Conversation
["hash", { alg, type, body }] writes the value serialized as `body`,
hashed with `alg`, as `type`; reading yields the digest. crc32 and
crc32c are built in, anything else goes through node's crypto and is a
Buffer. A CRC written into a signed type takes its two's complement.
The compiled writer needs the size of the body before it can serialize
it, which no writer could get at until now: WriteCompiler.callTypeSize
and SizeOfCompiler.callTypeWrite generate code with the sibling compiler
in the current scope and run it against that compiler's context.
SizeOfCompiler now remembers fixed-size natives so hashes into a
fixed-width type are sized without hashing.
| code += `;((buffer) => ${compiler.callTypeWrite('value', body, '0')})(bodyBuffer)\n` | ||
| code += `const hash = hashDigest(${JSON.stringify(alg)}, bodyBuffer)\n` | ||
| code += 'return ' + size | ||
| return compiler.wrapCode(code) |
There was a problem hiding this comment.
Hashes are pretty much always fixed in size.
May be nicer to have ProtoDef spec support an explicit list of hashes (SHA1, SHA256, etc) rather than relying on whatever Node.js standard lib exposes.
That will allow hard codeing the hash byte length into a map without having to fake hash first and allows other non-JS ProtoDef implementations to support an explicit list to be spec complaint
| @@ -0,0 +1,39 @@ | |||
| const crypto = require('crypto') | |||
There was a problem hiding this comment.
I don't think this belongs in the lib, we should either import lib (like crc) or at least move the hashing code to src/datatypes/hash.js which would contain the interpreter code plus the hashing code
The latter + importing the crc lib could be best ; you already have to import crypto as we don't do inline SHA hashing or anything like that
| /** | ||
| * Generates code with another compiler inside this compiler's scope, so that | ||
| * field references resolve to the same variables, and binds it to that | ||
| * compiler's context. Natives are reachable through the context as well. | ||
| */ | ||
| callTypeIn (other, ctxName, generate) { | ||
| if (!other) throw new Error(`${ctxName} is only available when compiling with ProtoDefCompiler`) | ||
| const scopeStack = other.scopeStack | ||
| other.scopeStack = this.scopeStack | ||
| try { | ||
| const code = generate(other) | ||
| if (!isNaN(code)) return code | ||
| return `((ctx, native) => ${code})(ctx.${ctxName}, ctx.${ctxName})` | ||
| } finally { | ||
| other.scopeStack = scopeStack | ||
| } | ||
| } |
There was a problem hiding this comment.
Should not be this complicated, no need to generate anything at call time
Compile order should be fixed to something like sizeOf=>write=>read so write can always call sizeOf
| this.readCtx = readCtx | ||
| // Code from callTypeSize / callTypeWrite runs against the other context | ||
| writeCtx.sizeOfCtx = sizeOfCtx | ||
| sizeOfCtx.writeCtx = writeCtx |
There was a problem hiding this comment.
SizeOfCompiler shouldn't need to write to figure out the size. That creates a potential cyclic dependency loop.
But it is useful for the WriteCompiler to know size of type such as for writing length prefixes for strings/array/buffer, hash digest, etc. Only reason looks like we didn't have this already is you can size a string/buffer in JS stdlib instead of needing ProtoDef (Buffer.byteLength vs needing to call our own sizeOf functions)
| // Local variable to provide some context to eval() | ||
| const native = this.native // eslint-disable-line | ||
| const { PartialReadError } = require('./utils') // eslint-disable-line | ||
| const hashDigest = require('./hash').digest // eslint-disable-line |
There was a problem hiding this comment.
This is another codesmell, specific data types should not require injecting stuff like this into the pre compile step.
parameterizable types do create duplication but all the types have the same issue, so we shouldn't inject just for this
So that other JS code should be directly copied into the codegen step
Or figure out way to use native/context type, but that would require looking at making them parameterizable
or split the code between a 'parameterizable' type with a parameterizable part that calls some native/context function
2 participants
Implements the
hashdatatype proposed in PrismarineJS/prismarine-item#184 (PrismarineJS/prismarine-item#184 (comment)):Write serializes the value as
body, hashes the bytes and writes the digest astype; read yields the digest.crc32andcrc32care implemented insrc/hash.js(thecrcpackage has no CRC32C, which is what Minecraft uses); any otheralggoes through node'scryptoand produces a Buffer. A CRC written into a signed type such asi32is written in two's complement, so it reads back as Java would produce it.Compiler
A compiled writer had no way to size a nested value, so it couldn't serialize the body into a scratch buffer (#169 is about the same gap). This adds
WriteCompiler.callTypeSize(value, type)andSizeOfCompiler.callTypeWrite(value, type, offsetExpr), which generate code with the sibling compiler in the current scope (socompareToreferences resolve to the same variables) and run it against that compiler's context, exposed asctx.sizeOfCtx/ctx.writeCtxlike #169 does. Documented indoc/compiler.md.SizeOfCompileralso remembers fixed-size natives, so a hash intoi32/lu32is sized without hashing.Tests
test/misc.js, interpreted and compiled: the CRC check values, signed output, avarintdigest, sha256, a body that switches on a field of the enclosing container, and a hash over a list of hashes.Schema and docs are in the ProtoDef submodule: ProtoDef-io/ProtoDef#65 (the submodule pointer here targets that branch).
Consumer: the HashOps encoding in PrismarineJS/prismarine-item#184 is built on this type.