Skip to content

fix: Fail on infinite recursion in encode.js #2099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: alpha
Choose a base branch
from
21 changes: 21 additions & 0 deletions src/CryptoUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Helper function that turns a string into a unique 53-bit hash.
* @ref https://stackoverflow.com/a/52171480/6456163

Check failure on line 3 in src/CryptoUtils.js

View workflow job for this annotation

GitHub Actions / build (Node 18, 18.19.0)

Invalid JSDoc tag name "ref"

Check failure on line 3 in src/CryptoUtils.js

View workflow job for this annotation

GitHub Actions / build (Node 20, 20.10.0)

Invalid JSDoc tag name "ref"
* @param {string} str
* @param {number} seed
* @returns {number}
*/
export const cyrb53 = (str, seed = 0) => {
let h1 = 0xdeadbeef ^ seed, h2 = 0x41c6ce57 ^ seed;
for (let i = 0, ch; i < str.length; i++) {
ch = str.charCodeAt(i);
h1 = Math.imul(h1 ^ ch, 2654435761);
h2 = Math.imul(h2 ^ ch, 1597334677);
}
h1 = Math.imul(h1 ^ (h1 >>> 16), 2246822507);
h1 ^= Math.imul(h2 ^ (h2 >>> 13), 3266489909);
h2 = Math.imul(h2 ^ (h2 >>> 16), 2246822507);
h2 ^= Math.imul(h1 ^ (h1 >>> 13), 3266489909);

return 4294967296 * (2097151 & h2) + (h1 >>> 0);
};
Comment on lines +1 to +21
Copy link
Member

@mtrezza mtrezza Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot accept this algo (or derivations thereof) due to its problematic usage rights situation. The author has claimed to have made this "public domain", but such a claim is invalid in some jurisdictions. I have not found an OS license attributed to this code by the author. If you have found that anywhere, please provide the link.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this solution be satisfactory?

bryc/code#23 (comment)

Copy link
Member

@mtrezza mtrezza Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort, but unfortunately the underlying issue is still unaddressed. I appreciate the author's rooting for public domain contributions. However, the issue is a legal one. Unless there's an official OS license attributed to the code, we cannot accept it. We also cannot accept any "self-written", non-official OS license on similar grounds.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtrezza is the MIT license addition satisfactory?

Copy link
Member

@mtrezza mtrezza Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but SO is not sufficient as source or link to the author. Has this been published anywhere on GitHub, and can this be added as a dependency? Because if we add code like this it won't be maintained by the author and the maintenance is on us. I'm not sure whether we should maintain this piece of code if there are libs for hash functions out there that are regularly maintained.

Why is this hash code even required in this PR? Are there no native Node hash functions we can use instead? This looks odd.

2 changes: 1 addition & 1 deletion src/ParseOp.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class SetOp extends Op {
}

toJSON(offline?: boolean) {
return encode(this._value, false, true, undefined, offline);
return encode(this._value, false, true, undefined, offline, 0);
}
}

Expand Down
9 changes: 0 additions & 9 deletions src/__tests__/ParseObject-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3878,13 +3878,4 @@ describe('ParseObject pin', () => {
'Traversing object failed due to high number of recursive calls, likely caused by circular reference within object.'
);
});

it('throws error for infinite recursion', () => {
const circularObject = {};
circularObject.circularReference = circularObject;

expect(() => {
encode(circularObject, false, false, [], false);
}).toThrowError('Encoding object failed due to high number of recursive calls, likely caused by circular reference within object.');
});
});
9 changes: 9 additions & 0 deletions src/__tests__/encode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,13 @@ describe('encode', () => {
str: 'abc',
});
});

it('handles circular references', () => {
const circularObject = {};
circularObject.circularReference = circularObject;

expect(() => {
encode(circularObject, false, false, [], false);
}).not.toThrow();
});
});
81 changes: 48 additions & 33 deletions src/encode.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import ParsePolygon from './ParsePolygon';
import ParseObject from './ParseObject';
import { Op } from './ParseOp';
import ParseRelation from './ParseRelation';
import { cyrb53 } from './CryptoUtils';

const MAX_RECURSIVE_CALLS = 999;

Expand All @@ -18,18 +19,15 @@ function encode(
forcePointers: boolean,
seen: Array<mixed>,
offline: boolean,
counter: number = 0
counter: number,
initialValue: mixed
): any {
counter++;

if (counter > MAX_RECURSIVE_CALLS) {
const message = 'Encoding object failed due to high number of recursive calls, likely caused by circular reference within object.';
console.error(message);
console.error('Value causing potential infinite recursion:', value);
console.error('Disallow objects:', disallowObjects);
console.error('Force pointers:', forcePointers);
console.error('Seen:', seen);
console.error('Offline:', offline);
console.error('Value causing potential infinite recursion:', initialValue);

throw new Error(message);
}
Expand All @@ -38,73 +36,90 @@ function encode(
if (disallowObjects) {
throw new Error('Parse Objects not allowed here');
}
const seenEntry = value.id ? value.className + ':' + value.id : value;
const entryIdentifier = value.id ? value.className + ':' + value.id : value;
if (
forcePointers ||
!seen ||
seen.indexOf(seenEntry) > -1 ||
seen.includes(entryIdentifier) ||
value.dirty() ||
Object.keys(value._getServerData()).length < 1
Object.keys(value._getServerData()).length === 0
) {
if (offline && value._getId().startsWith('local')) {
return value.toOfflinePointer();
}
return value.toPointer();
}
seen = seen.concat(seenEntry);
seen.push(entryIdentifier);
return value._toFullJSON(seen, offline);
}
if (
} else if (
value instanceof Op ||
value instanceof ParseACL ||
value instanceof ParseGeoPoint ||
value instanceof ParsePolygon ||
value instanceof ParseRelation
) {
return value.toJSON();
}
if (value instanceof ParseFile) {
} else if (value instanceof ParseFile) {
if (!value.url()) {
throw new Error('Tried to encode an unsaved file.');
}
return value.toJSON();
}
if (Object.prototype.toString.call(value) === '[object Date]') {
} else if (Object.prototype.toString.call(value) === '[object Date]') {
if (isNaN(value)) {
throw new Error('Tried to encode an invalid date.');
}
return { __type: 'Date', iso: (value: any).toJSON() };
}
if (
} else if (
Object.prototype.toString.call(value) === '[object RegExp]' &&
typeof value.source === 'string'
) {
return value.source;
}

if (Array.isArray(value)) {
return value.map(v => {
return encode(v, disallowObjects, forcePointers, seen, offline, counter);
});
}

if (value && typeof value === 'object') {
} else if (Array.isArray(value)) {
return value.map(v => encode(v, disallowObjects, forcePointers, seen, offline, counter, initialValue));
} else if (value && typeof value === 'object') {
const output = {};
for (const k in value) {
output[k] = encode(value[k], disallowObjects, forcePointers, seen, offline, counter);
try {
// Attempts to get the name of the object's constructor
// Ref: https://stackoverflow.com/a/332429/6456163
Copy link
Member

@mtrezza mtrezza Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same topic as earlier. If you copy/pasted someone else's code from SO then we cannot accept it.

Edit: Now that I looked at the link, this is a basic JS feature, there is really no need to reference SO here.

However, the above still holds true. I'd ask you to review the whole PR at this point and identify any code that has been copied from somewhere else, so we can investigate whether we can accept it.

const name = value[k].name || value[k].constructor.name;
if (name && name != "undefined") {
if (seen.includes(name)) {
output[k] = value[k];
continue;
} else {
seen.push(name);
}
}
} catch (e) {
// Support anonymous functions by hashing the function body,
// preventing infinite recursion in the case of circular references
if (value[k] instanceof Function) {
const funcString = value[k].toString();
if (seen.includes(funcString)) {
output[k] = value[k];
continue;
} else {
const hash = cyrb53(funcString);
seen.push(hash);
}
}
}
output[k] = encode(value[k], disallowObjects, forcePointers, seen, offline, counter, initialValue);
}
return output;
} else {
return value;
}

return value;
}

export default function (
value: mixed,
disallowObjects?: boolean,
forcePointers?: boolean,
seen?: Array<mixed>,
offline?: boolean
offline?: boolean,
counter?: number,
initialValue?: mixed
): any {
return encode(value, !!disallowObjects, !!forcePointers, seen || [], offline, 0);
return encode(value, !!disallowObjects, !!forcePointers, seen || [], !!offline, counter || 0, initialValue || value);
}
15 changes: 5 additions & 10 deletions src/unsavedChildren.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ function traverse(
if (counter > MAX_RECURSIVE_CALLS) {
const message = 'Traversing object failed due to high number of recursive calls, likely caused by circular reference within object.';
console.error(message);
console.error('Object causing potential infinite recursion:', obj);
console.error('Encountered objects:', encountered);
console.error('Object causing potential infinite recursion:', obj);

throw new Error(message);
}
Expand Down Expand Up @@ -89,16 +89,11 @@ function traverse(
if (obj instanceof ParseRelation) {
return;
}
if (Array.isArray(obj)) {
obj.forEach(el => {
if (typeof el === 'object') {
traverse(el, encountered, shouldThrow, allowDeepUnsaved, counter);
if (Array.isArray(obj) || typeof obj === 'object') {
for (const k in obj) {
if (typeof obj[k] === 'object') {
traverse(obj[k], encountered, shouldThrow, allowDeepUnsaved, counter);
}
});
}
for (const k in obj) {
if (typeof obj[k] === 'object') {
traverse(obj[k], encountered, shouldThrow, allowDeepUnsaved, counter);
}
}
}
Loading