Injiver 1512 js claim 169 mapper #151
Conversation
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
… with backward compatibility Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
…methods Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
Signed-off-by: srikanth716 <srikanthsri7447@gmail.com>
WalkthroughThis PR introduces breaking API changes to the JavaScript module, separating the single mapper parameter into distinct keyMapper and valueMapper parameters for getMappedData and decodeMappedData functions. It adds comprehensive CLAIM_169 mapping constants, new utility modules for CBOR and mapper operations, refactors core encoding/decoding logic with enhanced CBOR handling, and substantially rewrites the test suite. Package version is bumped to 0.8.0 with repository URLs updated to the inji organization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/Readme.md (1)
69-71: Git clone URL still references old repository location.The example command references
mosip/pixelpassbut the repository has moved toinji/pixelpassbased on the package.json and gradle file updates. Update for consistency:Proposed fix
-git clone https://github.com/mosip/pixelpass.git && cd pixelpass && git checkout develop && cd js && npm i && cd example && npm i && npm start +git clone https://github.com/inji/pixelpass.git && cd pixelpass && git checkout develop && cd js && npm i && cd example && npm i && npm start
🤖 Fix all issues with AI agents
In `@js/src/index.js`:
- Around line 84-90: The current check `if (decodedCBORData)` incorrectly treats
valid CBOR values like 0, "", false, or null as falsy and falls back to
`textData`; update the conditional after
`cbor.decodeFirstSync(decompressedData)` to detect successful decoding by
checking for undefined specifically (e.g., `if (decodedCBORData !== undefined)
return JSON.stringify(decodedCBORData);`) so that valid falsy CBOR values
(including null) are returned correctly; keep the existing try/catch and
fallback to `textData` on errors.
- Around line 93-107: In decodeBinary, don’t UTF-8 decode the incoming binary
blob before handing it to JSZip: decoding corrupts ZIP bytes. Instead, detect
the ZIP header by inspecting the raw bytes (e.g., check the first few bytes of
the provided ArrayBuffer/Uint8Array against ZIP_HEADER) and call JSZip.loadAsync
with the original binary (ArrayBuffer/Uint8Array) rather than the TextDecoder
output; then locate DEFAULT_ZIP_FILE_NAME and return await file.async("text") as
before, preserving the existing error paths.
In `@js/src/utils/cborUtils.js`:
- Around line 169-171: The helper decodeFromBase64UrlFormat uses Buffer.from
with the 'base64url' encoding which requires Node.js >=14.18.0; update
package.json by adding an "engines" field specifying "node": ">=14.18.0" so
package metadata reflects the runtime requirement and prevents installs on
unsupported Node versions (add the engines object and optionally update README
or CI to enforce it).
In `@js/src/utils/mapperUtils.js`:
- Around line 6-16: The current normalizedValueMapper mapping uses typeof field
=== "string" && field.toLowerCase(), which yields the boolean false for
non-string fields and creates a "false" key; change this to reliably produce a
string key by using String(field).toLowerCase() (or alternatively filter out
non-string fields) when building normalizedValueMapper from valueMapper so keys
are always normalized strings; update the mapping expression that generates the
outer key (where field and mappings are destructured) to use
String(field).toLowerCase() and keep the inner src normalization as-is.
In `@js/test/decodeBinary.test.js`:
- Around line 5-12: The test corrupts ZIP binary by using JSZip.generateAsync({
type: "string" }) and then TextEncoder().encode(), which treats binary as UTF-8
text; change the ZIP generation to use generateAsync({ type: "uint8array" }) and
pass that Uint8Array directly into decodeBinary (remove TextEncoder().encode
usage) so decodeBinary receives raw binary bytes; update the test case that
calls decodeBinary to accept the Uint8Array returned by JSZip.
- Around line 20-37: Tests "decode ZIP with multiple files" and "decode empty
ZIP entry" pass string data to decodeBinary via TextEncoder; change
JSZip.generateAsync to return a Uint8Array and pass that directly to
decodeBinary. Update the two tests to call zip.generateAsync({ type:
"uint8array" }) and pass the returned value into decodeBinary (calling
decodeBinary(data) instead of decodeBinary(new TextEncoder().encode(data))),
keeping the rest of the test assertions the same and referencing the
decodeBinary function and JSZip.generateAsync usage.
In `@js/test/decodeMappedData.test.js`:
- Around line 53-60: The test currently calls getMappedData(decodedData, [], [],
true) using empty arrays for the mappers; change those to empty objects so the
call is getMappedData(decodedData, {}, {}, true) to match normalizeMappers'
expected mapper shape and to be consistent with other tests (see getMappedData
and normalizeMappers references).
🧹 Nitpick comments (6)
js/test/generateQRCode.test.js (1)
4-25: Test coverage for happy paths is adequate.The tests verify that
generateQRCodereturns valid base64 PNG data URLs for various inputs and ECC levels. Consider adding tests for edge cases likenull/undefinedinputs to verify error handling behavior, and optionally a round-trip test that decodes the generated QR to verify content integrity.js/test/generateQRData.test.js (1)
17-20: Consider asserting more specific behavior for empty string.The test only verifies that the result is non-empty. Consider also verifying that decoding the result returns an empty string to ensure round-trip correctness.
Suggested improvement
test("handles empty string", () => { const result = generateQRData(""); expect(result.length).toBeGreaterThan(0); + expect(decode(result)).toBe(""); });js/test/toJson.test.js (1)
43-58: Clarify expected Map key behavior.When CBOR Maps with integer keys are converted to JSON objects, the keys become strings due to JavaScript's object key coercion. The expectation
{ 1: "one", 2: "two" }is equivalent to{ "1": "one", "2": "two" }in JavaScript. Consider adding a comment to clarify this behavior for future maintainers.js/test/decode.test.js (1)
37-44: Consider extracting test data to a fixture file.The CWT data strings are quite long and reduce test readability. Consider moving these to a separate test fixtures file.
js/src/utils/cborUtils.js (1)
62-66: Redundant null handling branch.Line 62-63 explicitly handles
value === null, but this case is already excluded by the condition at line 60 (typeof value === "object" && value !== null). Theelsebranch at lines 64-66 would handle primitives includingnullwhen it falls through.♻️ Simplify by removing redundant branch
} else if (typeof value === "object" && value !== null) { processedValue = replaceKeysAtDepth(value, mapper, depth, nextDepth); - } else if (value === null) { - processedValue = null; } else { processedValue = value; }js/src/index.js (1)
52-57: Consider removing console.error from library code.Line 53 logs
"Data is not JSON"to stderr. Library code typically should not log directly, allowing consumers to handle errors as they see fit. The error is silently caught and fallback behavior is applied, so this message could be confusing for library users.♻️ Remove console.error or make it optional
} catch (e) { - console.error("Data is not JSON"); compressedData = pako.deflate(data, { level: DEFAULT_ZLIB_COMPRESSION_LEVEL, });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
js/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
js/Readme.mdjs/package.jsonjs/src/index.jsjs/src/shared/Constants.jsjs/src/utils/cborUtils.jsjs/src/utils/mapperUtils.jsjs/test/PixelPass.test.jsjs/test/UtilsTest.test.jsjs/test/decode.test.jsjs/test/decodeBinary.test.jsjs/test/decodeMappedData.test.jsjs/test/generateQRCode.test.jsjs/test/generateQRData.test.jsjs/test/getMappedData.test.jsjs/test/toJson.test.jskotlin/PixelPass/publish-artifact.gradle
💤 Files with no reviewable changes (1)
- js/test/PixelPass.test.js
🧰 Additional context used
🧬 Code graph analysis (10)
js/test/decodeMappedData.test.js (3)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)js/test/UtilsTest.test.js (5)
require(1-6)require(8-12)require(14-17)map(315-322)mapper(290-290)js/src/utils/cborUtils.js (1)
require(1-8)
js/test/toJson.test.js (2)
js/src/index.js (4)
cbor(18-18)require(1-14)require(20-25)require(26-26)js/src/utils/cborUtils.js (1)
require(1-8)
js/test/UtilsTest.test.js (4)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)js/src/utils/cborUtils.js (5)
require(1-8)result(43-43)result(87-87)data(12-12)data(24-24)js/test/toJson.test.js (1)
require(2-2)js/src/utils/mapperUtils.js (2)
result(33-33)result(79-79)
js/src/utils/cborUtils.js (2)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)js/test/UtilsTest.test.js (3)
require(1-6)require(8-12)require(14-17)
js/test/decode.test.js (6)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)js/test/decodeMappedData.test.js (1)
require(1-1)js/test/UtilsTest.test.js (3)
require(1-6)require(8-12)require(14-17)js/src/utils/cborUtils.js (1)
require(1-8)js/test/decodeBinary.test.js (1)
require(1-1)js/test/generateQRData.test.js (1)
require(1-1)
js/test/generateQRCode.test.js (2)
kotlin/PixelPass/src/commonMain/kotlin/io/mosip/pixelpass/PixelPass.kt (1)
generateQRCode(34-38)js/src/shared/Constants.js (1)
ECC(1-1)
js/test/decodeBinary.test.js (2)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)kotlin/PixelPass/src/commonMain/kotlin/io/mosip/pixelpass/PixelPass.kt (1)
decodeBinary(56-71)
js/src/utils/mapperUtils.js (1)
js/test/UtilsTest.test.js (25)
keyMapper(177-177)keyMapper(190-190)keyMapper(221-221)keyMapper(243-243)keyMapper(256-256)keyMapper(273-276)data(255-255)result(32-32)result(52-52)result(62-62)result(68-68)result(82-82)result(97-97)result(113-113)result(127-127)result(143-143)result(159-159)result(179-179)result(192-192)result(211-211)result(223-223)arr(189-189)arr(200-209)arr(220-220)arr(234-242)
js/test/getMappedData.test.js (3)
js/src/index.js (3)
require(1-14)require(20-25)require(26-26)js/test/UtilsTest.test.js (5)
require(1-6)require(8-12)require(14-17)data(255-255)map(315-322)js/src/utils/cborUtils.js (3)
require(1-8)data(12-12)data(24-24)
js/src/index.js (1)
js/src/utils/cborUtils.js (2)
data(12-12)data(24-24)
🪛 LanguageTool
js/Readme.md
[style] ~9-~9: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ata→ Gives back CBOR-encoded data. - Given a CBOR-encoded data as byte array →de...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... → decode → Gives back JSON String. - Given data as byteArray → decodeBinary → Gi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...codeBinary→ Gives back JSON String. - Given a JSON and Mapper →getMappedData` → G...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ck CBOR-encoded data or mapped JSON. - Given a CBOR-encoded data and Mapper → `decod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~118-~118: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: .... ### decode( data ) - data` - Data needs to be decoded and decompressed without hea...
(REP_NEED_TO_VB)
🔇 Additional comments (25)
kotlin/PixelPass/publish-artifact.gradle (2)
32-52: Repository URL updates look correct.The SCM and project URLs are consistently updated to the new
inji/pixelpassrepository. Note thatgroup = "io.mosip"and developer metadata still reference "Mosip" - this appears intentional (organization identity vs. repository location), but verify this is the desired state for published artifact metadata.
126-165: jarRelease publication URLs updated consistently.The URL changes in the jarRelease publication mirror those in the aar publication, maintaining consistency across both artifact types.
js/package.json (1)
2-7: Version bump and repository URLs updated appropriately.The version bump to
0.8.0aligns with the breaking API changes documented in the Readme. The package name remains@mosip/pixelpasswhile repository URLs point toinji/pixelpass- this is acceptable but worth confirming this is intentional, as it may cause confusion for users navigating between npm and GitHub.js/Readme.md (2)
15-42: Breaking changes documentation is well-structured.The breaking changes and API contract changes sections clearly document the signature changes for
getMappedDataanddecodeMappedData. This will help users migrate from previous versions.
237-262: Migration guide is helpful.The side-by-side old vs. new API examples and the list of key differences (array wrapper, new valueMapper parameter, JSON string return type) provide clear migration guidance.
js/src/utils/mapperUtils.js (2)
20-76: Recursive transformation logic looks correct.The
toMapWithKeyAndValueMapperfunction properly handles null, arrays, non-objects, and nested structures through mutual recursion withtoListWithKeyAndValueMapper. The early returns and type checks are appropriate.
78-107: Array transformation handles all value types appropriately.The function correctly preserves nulls, recursively processes nested objects and arrays, and passes through primitives unchanged.
js/test/generateQRData.test.js (3)
1-6: LGTM!Clean import and straightforward test for basic string encoding. The expected output format is verified against a known value.
8-10: LGTM!Good test for JSON-to-CBOR QR encoding with a concrete expected value.
22-31: LGTM!Good coverage for complex nested JSON with round-trip verification and header prefix functionality.
js/test/toJson.test.js (2)
1-21: LGTM!Clean test setup encoding JSON to CBOR via
cbor-web, converting to base64url, and verifying round-trip throughtoJson. Good coverage for basic object decoding.
60-93: LGTM!Good coverage for edge cases: root-level arrays, null values, boolean primitives, and error handling for invalid base64url input.
js/test/decode.test.js (2)
1-16: LGTM!Good coverage for basic decoding scenarios: simple base45 strings, CBOR-encoded QR data, and array round-trips through
generateQRData/decode.
18-35: LGTM!Proper coverage for edge cases: null values, booleans, text fallback when CBOR fails, and error handling for invalid base45 input.
js/test/getMappedData.test.js (4)
1-17: LGTM!Good basic test for object field mapping with the new separated mapper parameters.
19-25: LGTM!Good test verifying CBOR hex output when
cborEnableflag is set. Passingundefinedas valueMapper correctly tests the optional parameter.
49-87: LGTM!Comprehensive round-trip test for CLAIM_169 mapping with CBOR encoding and decoding. Good coverage of nested biometric data structures.
44-47: This is correct API usage with proper null handling.Calling
getMappedDatawithout explicit mappers is intended behavior—the function has default parameters (keyMapper = CLAIM_169_KEY_MAPPER,valueMapper = CLAIM_169_VALUE_MAPPER). Unmapped keys (like"a") fall back to their original names, and null values are explicitly preserved in the implementation. The test expectation is accurate.js/test/decodeBinary.test.js (1)
14-18: LGTM!Good error handling test verifying that non-ZIP data throws the expected error message.
js/test/UtilsTest.test.js (1)
1-332: Well-structured test coverage for utility functions.The test suite provides comprehensive coverage including:
- Edge cases for
translateToJsonwith nested Maps and mixed types- Primitive value handling in
replaceKeysAtDepth- Multiple edge cases for
replaceValuesForClaim169(NaN, missing mappings, null values, non-object biometrics)- Mapper utilities with nested arrays, null values, and mixed types
- Integration tests validating public API behavior
The organization with clear section comments enhances maintainability.
js/src/shared/Constants.js (2)
56-95: Note:Data sub formatvalue mappings are context-dependent.The
CLAIM_169_VALUE_MAPPER["Data sub format"]has overlapping numeric codes (e.g.,0maps to both"PNG","Fingerprint Template ANSI 378", and"WAV"). This is intentional as the sub-format meaning depends on the parentData format.The reverse mapping in
CLAIM_169_BIOMETRIC_SUB_FORMAT_REVERSE_VALUE_MAPPER(lines 157-170) correctly disambiguates by keying on the parent format type. This design is appropriate for the domain.
13-54: CLAIM_169_KEY_MAPPER constants look correct.The key mapper provides comprehensive mappings for identity and biometric fields with appropriate numeric codes. The structure supports both root-level fields (1-18) and biometric-specific fields (50-65, 0-3 for nested data).
js/src/index.js (2)
28-41: New toJson function is well-implemented.The function properly validates input type, handles base64url decoding, CBOR parsing, and JSON translation with appropriate error wrapping. Good defensive coding.
134-182: decodeMappedData handles depth-aware mapping correctly.The function properly:
- Validates null/undefined input
- Handles array inputs recursively
- Attempts CBOR decoding first, falls back to JSON
- Validates keyMapper is an array for depth-aware decoding
- Applies depth-based key replacement and optional value mapping
The error messages are descriptive and the flow is well-structured.
js/src/utils/cborUtils.js (1)
101-112: Potential TypeError when spreading null biometric value.At line 104,
{ ...result[nestedKey] }will throw a TypeError ifresult[nestedKey]isnull. The early return at line 102 only checks forundefined, allowingnullvalues to reach the spread operator.🐛 Proposed fix
CLAIM_169_BIOMETRIC_KEYS.forEach((nestedKey) => { - if (result[nestedKey] === undefined) return; + if (result[nestedKey] == null) return; const nestedObject = { ...result[nestedKey] };Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try { | ||
| const decodedCBORData = cbor.decodeFirstSync(decompressedData); | ||
| if (decodedCBORData) return JSON.stringify(decodedCBORData); | ||
| return textData; | ||
| } catch (e) { | ||
| return textData; | ||
| } |
There was a problem hiding this comment.
Falsy CBOR values would incorrectly fall through to text return.
Line 86 checks if (decodedCBORData) which would return textData for valid CBOR values like 0, "", false, or null. This could cause incorrect decoding behavior for payloads containing these values.
🐛 Proposed fix
try {
const decodedCBORData = cbor.decodeFirstSync(decompressedData);
- if (decodedCBORData) return JSON.stringify(decodedCBORData);
- return textData;
+ return JSON.stringify(decodedCBORData);
} catch (e) {
return textData;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const decodedCBORData = cbor.decodeFirstSync(decompressedData); | |
| if (decodedCBORData) return JSON.stringify(decodedCBORData); | |
| return textData; | |
| } catch (e) { | |
| return textData; | |
| } | |
| try { | |
| const decodedCBORData = cbor.decodeFirstSync(decompressedData); | |
| return JSON.stringify(decodedCBORData); | |
| } catch (e) { | |
| return textData; | |
| } |
🤖 Prompt for AI Agents
In `@js/src/index.js` around lines 84 - 90, The current check `if
(decodedCBORData)` incorrectly treats valid CBOR values like 0, "", false, or
null as falsy and falls back to `textData`; update the conditional after
`cbor.decodeFirstSync(decompressedData)` to detect successful decoding by
checking for undefined specifically (e.g., `if (decodedCBORData !== undefined)
return JSON.stringify(decodedCBORData);`) so that valid falsy CBOR values
(including null) are returned correctly; keep the existing try/catch and
fallback to `textData` on errors.
| async function decodeBinary(data) { | ||
| let decodedData = new TextDecoder("utf-8").decode(data); | ||
| if (decodedData.startsWith(ZIP_HEADER)){ | ||
| return (await JSZip.loadAsync(decodedData)).file(DEFAULT_ZIP_FILE_NAME).async("text") | ||
| }else { | ||
| throw new Error("Unsupported binary file type"); | ||
| let decodedData = new TextDecoder("utf-8").decode(data); | ||
| if (decodedData.startsWith(ZIP_HEADER)) { | ||
| const zip = await JSZip.loadAsync(decodedData); | ||
| const file = zip.file(DEFAULT_ZIP_FILE_NAME); | ||
| if (!file) { | ||
| throw new Error( | ||
| `File '${DEFAULT_ZIP_FILE_NAME}' not found in ZIP archive` | ||
| ); | ||
| } | ||
| return file.async("text"); | ||
| } else { | ||
| throw new Error("Unsupported binary file type"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential issue with JSZip.loadAsync receiving decoded string for binary ZIP data.
At line 94, binary data is decoded to a UTF-8 string, then at line 96, this string is passed to JSZip.loadAsync(). For binary ZIP files, decoding to UTF-8 can corrupt the data since ZIP is binary format.
JSZip.loadAsync should receive the original binary data (Uint8Array/ArrayBuffer) for reliable ZIP extraction.
🐛 Proposed fix
async function decodeBinary(data) {
- let decodedData = new TextDecoder("utf-8").decode(data);
- if (decodedData.startsWith(ZIP_HEADER)) {
- const zip = await JSZip.loadAsync(decodedData);
+ // Check ZIP header from first 2 bytes
+ const header = new TextDecoder("utf-8").decode(data.slice(0, 2));
+ if (header.startsWith(ZIP_HEADER)) {
+ const zip = await JSZip.loadAsync(data);
const file = zip.file(DEFAULT_ZIP_FILE_NAME);🤖 Prompt for AI Agents
In `@js/src/index.js` around lines 93 - 107, In decodeBinary, don’t UTF-8 decode
the incoming binary blob before handing it to JSZip: decoding corrupts ZIP
bytes. Instead, detect the ZIP header by inspecting the raw bytes (e.g., check
the first few bytes of the provided ArrayBuffer/Uint8Array against ZIP_HEADER)
and call JSZip.loadAsync with the original binary (ArrayBuffer/Uint8Array)
rather than the TextDecoder output; then locate DEFAULT_ZIP_FILE_NAME and return
await file.async("text") as before, preserving the existing error paths.
| function decodeFromBase64UrlFormat(content) { | ||
| return Buffer.from(content, 'base64url'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check package.json for Node.js version requirements
if [ -f "package.json" ]; then
echo "=== package.json engines field ==="
cat package.json | jq '.engines' 2>/dev/null || echo "No engines field found"
echo ""
echo "=== Full package.json (first 100 lines) ==="
head -100 package.json
fiRepository: inji/pixelpass
Length of output: 40
🌐 Web query:
Node.js Buffer.from base64url encoding when introduced minimum version
💡 Result:
The "base64url" encoding for Buffer (e.g., Buffer.from(..., 'base64url')) was introduced in Node.js v15.7.0 and backported to the v14 line in v14.18.0 — so the minimum supported release is v14.18.0. [1][2]
Sources:
- Node.js Buffer docs (history: "v15.7.0, v14.18.0 Introduced
base64urlencoding"). [1] - GitHub issue tracking support for base64url. [2]
🏁 Script executed:
#!/bin/bash
# Read package.json
cat package.jsonRepository: inji/pixelpass
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Find package.json files in the repository
fd -t f 'package.json' -x cat {} \;Repository: inji/pixelpass
Length of output: 1155
Add Node.js version requirement to package.json.
The function uses Buffer.from(content, 'base64url'), which requires Node.js 14.18.0 or later. Add an engines field to package.json to declare this minimum version:
"engines": {
"node": ">=14.18.0"
}🤖 Prompt for AI Agents
In `@js/src/utils/cborUtils.js` around lines 169 - 171, The helper
decodeFromBase64UrlFormat uses Buffer.from with the 'base64url' encoding which
requires Node.js >=14.18.0; update package.json by adding an "engines" field
specifying "node": ">=14.18.0" so package metadata reflects the runtime
requirement and prevents installs on unsupported Node versions (add the engines
object and optionally update README or CI to enforce it).
| normalizedValueMapper: Object.fromEntries( | ||
| Object.entries(valueMapper).map(([field, mappings]) => [ | ||
| typeof field === "string" && field.toLowerCase(), | ||
| Object.fromEntries( | ||
| Object.entries(mappings).map(([src, mapped]) => [ | ||
| String(src).toLowerCase(), | ||
| mapped, | ||
| ]) | ||
| ), | ||
| ]) | ||
| ), |
There was a problem hiding this comment.
Potential bug: non-string field produces "false" key.
When field is not a string, typeof field === "string" && field.toLowerCase() evaluates to false, which becomes the string key "false" in normalizedValueMapper. This could cause unexpected behavior or key collisions.
Proposed fix
normalizedValueMapper: Object.fromEntries(
Object.entries(valueMapper).map(([field, mappings]) => [
- typeof field === "string" && field.toLowerCase(),
+ typeof field === "string" ? field.toLowerCase() : String(field).toLowerCase(),
Object.fromEntries(
Object.entries(mappings).map(([src, mapped]) => [
String(src).toLowerCase(),
mapped,
])
),
])
),🤖 Prompt for AI Agents
In `@js/src/utils/mapperUtils.js` around lines 6 - 16, The current
normalizedValueMapper mapping uses typeof field === "string" &&
field.toLowerCase(), which yields the boolean false for non-string fields and
creates a "false" key; change this to reliably produce a string key by using
String(field).toLowerCase() (or alternatively filter out non-string fields) when
building normalizedValueMapper from valueMapper so keys are always normalized
strings; update the mapping expression that generates the outer key (where field
and mappings are destructured) to use String(field).toLowerCase() and keep the
inner src normalization as-is.
| test("decode ZIP binary data", async () => { | ||
| const zip = new JSZip(); | ||
| zip.file("certificate.json", "Hello World!!"); | ||
| const data = await zip.generateAsync({ type: "string" }); | ||
|
|
||
| const result = await decodeBinary(new TextEncoder().encode(data)); | ||
| expect(result).toBe("Hello World!!"); | ||
| }); |
There was a problem hiding this comment.
Potential binary data corruption using TextEncoder on ZIP data.
Using JSZip.generateAsync({ type: "string" }) produces a binary string, and then TextEncoder().encode() treats it as UTF-8 text. This can corrupt binary data containing non-ASCII bytes (values > 127). The test may pass coincidentally for small ASCII-compatible content.
Use type: "uint8array" directly to avoid encoding issues.
Suggested fix
test("decode ZIP binary data", async () => {
const zip = new JSZip();
zip.file("certificate.json", "Hello World!!");
- const data = await zip.generateAsync({ type: "string" });
+ const data = await zip.generateAsync({ type: "uint8array" });
- const result = await decodeBinary(new TextEncoder().encode(data));
+ const result = await decodeBinary(data);
expect(result).toBe("Hello World!!");
});🤖 Prompt for AI Agents
In `@js/test/decodeBinary.test.js` around lines 5 - 12, The test corrupts ZIP
binary by using JSZip.generateAsync({ type: "string" }) and then
TextEncoder().encode(), which treats binary as UTF-8 text; change the ZIP
generation to use generateAsync({ type: "uint8array" }) and pass that Uint8Array
directly into decodeBinary (remove TextEncoder().encode usage) so decodeBinary
receives raw binary bytes; update the test case that calls decodeBinary to
accept the Uint8Array returned by JSZip.
| test("decode ZIP with multiple files", async () => { | ||
| const zip = new JSZip(); | ||
| zip.file("certificate.json", "Cert"); | ||
| zip.file("other.txt", "Other"); | ||
| const data = await zip.generateAsync({ type: "string" }); | ||
|
|
||
| const result = await decodeBinary(new TextEncoder().encode(data)); | ||
| expect(result).toBe("Cert"); | ||
| }); | ||
|
|
||
| test("decode empty ZIP entry", async () => { | ||
| const zip = new JSZip(); | ||
| zip.file("certificate.json", ""); | ||
| const data = await zip.generateAsync({ type: "string" }); | ||
|
|
||
| const result = await decodeBinary(new TextEncoder().encode(data)); | ||
| expect(result).toBe(""); | ||
| }); |
There was a problem hiding this comment.
Apply the same binary data fix to remaining tests.
Same issue as above - use type: "uint8array" directly instead of string conversion via TextEncoder.
Suggested fix for multiple files test
test("decode ZIP with multiple files", async () => {
const zip = new JSZip();
zip.file("certificate.json", "Cert");
zip.file("other.txt", "Other");
- const data = await zip.generateAsync({ type: "string" });
+ const data = await zip.generateAsync({ type: "uint8array" });
- const result = await decodeBinary(new TextEncoder().encode(data));
+ const result = await decodeBinary(data);
expect(result).toBe("Cert");
});
test("decode empty ZIP entry", async () => {
const zip = new JSZip();
zip.file("certificate.json", "");
- const data = await zip.generateAsync({ type: "string" });
+ const data = await zip.generateAsync({ type: "uint8array" });
- const result = await decodeBinary(new TextEncoder().encode(data));
+ const result = await decodeBinary(data);
expect(result).toBe("");
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("decode ZIP with multiple files", async () => { | |
| const zip = new JSZip(); | |
| zip.file("certificate.json", "Cert"); | |
| zip.file("other.txt", "Other"); | |
| const data = await zip.generateAsync({ type: "string" }); | |
| const result = await decodeBinary(new TextEncoder().encode(data)); | |
| expect(result).toBe("Cert"); | |
| }); | |
| test("decode empty ZIP entry", async () => { | |
| const zip = new JSZip(); | |
| zip.file("certificate.json", ""); | |
| const data = await zip.generateAsync({ type: "string" }); | |
| const result = await decodeBinary(new TextEncoder().encode(data)); | |
| expect(result).toBe(""); | |
| }); | |
| test("decode ZIP with multiple files", async () => { | |
| const zip = new JSZip(); | |
| zip.file("certificate.json", "Cert"); | |
| zip.file("other.txt", "Other"); | |
| const data = await zip.generateAsync({ type: "uint8array" }); | |
| const result = await decodeBinary(data); | |
| expect(result).toBe("Cert"); | |
| }); | |
| test("decode empty ZIP entry", async () => { | |
| const zip = new JSZip(); | |
| zip.file("certificate.json", ""); | |
| const data = await zip.generateAsync({ type: "uint8array" }); | |
| const result = await decodeBinary(data); | |
| expect(result).toBe(""); | |
| }); |
🤖 Prompt for AI Agents
In `@js/test/decodeBinary.test.js` around lines 20 - 37, Tests "decode ZIP with
multiple files" and "decode empty ZIP entry" pass string data to decodeBinary
via TextEncoder; change JSZip.generateAsync to return a Uint8Array and pass that
directly to decodeBinary. Update the two tests to call zip.generateAsync({ type:
"uint8array" }) and pass the returned value into decodeBinary (calling
decodeBinary(data) instead of decodeBinary(new TextEncoder().encode(data))),
keeping the rest of the test assertions the same and referencing the
decodeBinary function and JSZip.generateAsync usage.
| test("decode CBOR hex string for claim-169 mapped full JSON", () => { | ||
| const hex = | ||
| "ac6249446a333931383539323433386756657273696f6e0a6946756c6c204e616d656c4a616e61726468616e2042536d44617465206f662042697274686a30342d31382d313938346647656e646572644d616c65674164647265737378294e657720486f7573652c204e656172204d6574726f204c696e652c2042656e67616c7572752c204b4168456d61696c204944756a616e61726468616e406578616d706c652e636f6d6c50686f6e65204e756d6265726d2b3931393837363534333231306b4e6174696f6e616c69747962494e6446616365a3644461746164353234396b4461746120666f726d617465496d6167656f446174612073756220666f726d617463504e4765566f696365a3644461746164353234396b4461746120666f726d617465536f756e646f446174612073756220666f726d6174635741566568656c6c6f65776f726c64"; | ||
| const decode = decodeMappedData(hex); | ||
| const decodedData = JSON.parse(decode); | ||
| const hexData = getMappedData(decodedData, [], [], true); | ||
| expect(hexData).toBe(hex); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd js && find . -name "*.js" -type f | grep -E "(index|decodeMappedData)" | head -20Repository: inji/pixelpass
Length of output: 104
🏁 Script executed:
# Check the test file structure
head -70 js/test/decodeMappedData.test.jsRepository: inji/pixelpass
Length of output: 2531
🏁 Script executed:
# Check the index.js file around the getMappedData function
cat -n js/src/index.js | head -150Repository: inji/pixelpass
Length of output: 5144
🏁 Script executed:
# Find and read the mapperUtils.js file
find js -name "mapperUtils.js" -type fRepository: inji/pixelpass
Length of output: 85
🏁 Script executed:
# Read the mapperUtils file
cat -n js/src/utils/mapperUtils.jsRepository: inji/pixelpass
Length of output: 3704
🏁 Script executed:
# Check if the test file has any explicit expectations about mapper types
grep -n "getMappedData" js/test/decodeMappedData.test.jsRepository: inji/pixelpass
Length of output: 266
🏁 Script executed:
# Check the Constants file to see how default mappers are defined
grep -A 5 "CLAIM_169_KEY_MAPPER\|CLAIM_169_VALUE_MAPPER" js/src/shared/Constants.js | head -30Repository: inji/pixelpass
Length of output: 297
🏁 Script executed:
# Verify what happens when Object.entries is called on an empty array
node <<'EOF'
const arr = [];
console.log("Object.entries([]):", Object.entries(arr));
console.log("Object.fromEntries([]):", Object.fromEntries([]));
// Simulate normalizeMappers with empty array input
const keyMapper = [];
const normalized = Object.fromEntries(
Object.entries(keyMapper).map(([k, v]) => [k.toLowerCase(), v])
);
console.log("normalizeMappers result with []:", normalized);
console.log("Is it an object?", typeof normalized === "object");
console.log("Entries:", Object.entries(normalized));
EOFRepository: inji/pixelpass
Length of output: 177
Use empty objects {} instead of empty arrays [] for mapper parameters.
On line 58, getMappedData(decodedData, [], [], true) passes empty arrays for keyMapper and valueMapper. While these are functionally converted to empty objects by normalizeMappers, using explicit empty objects getMappedData(decodedData, {}, {}, true) is clearer and consistent with line 41 where {} is already used for the valueMapper parameter.
🤖 Prompt for AI Agents
In `@js/test/decodeMappedData.test.js` around lines 53 - 60, The test currently
calls getMappedData(decodedData, [], [], true) using empty arrays for the
mappers; change those to empty objects so the call is getMappedData(decodedData,
{}, {}, true) to match normalizeMappers' expected mapper shape and to be
consistent with other tests (see getMappedData and normalizeMappers references).
|
Duplicate PR, hence closing |
1 participant
Summary by CodeRabbit
Release Notes
New Features
toJsonutility for converting base64url-encoded data to JSON.Breaking Changes
getMappedDataanddecodeMappedDatamethod signatures; refer to migration guide in documentation.Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.