Skip to content
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

bug: Capacitor androidBridge.postMessage converting null to "..." #4798

Closed
chriswep opened this issue Jul 7, 2021 · 13 comments · Fixed by #4853
Closed

bug: Capacitor androidBridge.postMessage converting null to "..." #4798

chriswep opened this issue Jul 7, 2021 · 13 comments · Fixed by #4853
Assignees

Comments

@chriswep
Copy link

chriswep commented Jul 7, 2021

originally posted here but closed due to inactivity: #4746

the method safeStringify in

does not account for null values when checking for circular references (null has the type object as well). therefore [10,null,null,null] is converted to [10,null,'...','...'] (as posted in the original issue).

bug was introduced here: 0429905#diff-8afda3e2bce7fb91a14a66d96d63a89f112b852cf39223e6f820a1575cbdafebR175

@jcesarmobile jcesarmobile added the needs reproduction needs reproducible example to illustrate the issue label Jul 7, 2021
@Ionitron
Copy link
Collaborator

Ionitron commented Jul 7, 2021

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks!
Ionitron 💙

@Ionitron Ionitron added the needs reply needs reply from the user label Jul 7, 2021
@chriswep
Copy link
Author

chriswep commented Jul 7, 2021

paste this into console (this is the method used to post data to native android):

        const safeStringify = (value) => {
            const seen = new Set();
            return JSON.stringify(value, (_k, v) => {
                if (seen.has(v)) {
                    return '...';
                }
                if (typeof v === 'object') {
                    seen.add(v);
                }
                return v;
            });
        };
        
        safeStringify([null,null]);
        
        // output: "[null,\"...\"]"

@Ionitron Ionitron removed the needs reply needs reply from the user label Jul 7, 2021
@jcesarmobile
Copy link
Member

Sorry, I still need an app where it can be reproduced

@jcesarmobile jcesarmobile added the needs reply needs reply from the user label Jul 7, 2021
@chriswep
Copy link
Author

chriswep commented Jul 7, 2021

ok, someone posted one in the original issue: https://github.com/gabrielcursino/capacitor-sqlite-test

however i think you might save some time by just running the method directly, since it is obviously treating null values as circular object references. this is an issue wherever this method is used.

@Ionitron Ionitron removed the needs reply needs reply from the user label Jul 7, 2021
@jepiqueau
Copy link
Contributor

@jcesarmobile @chriswep you can find the app to reproduce the issue https://github.com/jepiqueau/capacitor-sqlite-test.git

@jepiqueau
Copy link
Contributor

@jcesarmobile @chriswep seems that if you modify in assets/native-bridge.js the constant safeStringify by

const safeStringify = (value) => {
    const seen = new Set();
    return JSON.stringify(value, (_k, v) => {
        if (typeof v === 'object') {
            seen.add(v);
        }
        return v;
    });
};

it works , but i do not know what is the impact of doing that on the rest of the code and what the reason to have these lines

                if (seen.has(v)) {
                    return '...';
                }

Hope this will help

@chriswep
Copy link
Author

chriswep commented Jul 8, 2021

@jepiqueau the purpose of the method is to remove circular references in an object since JSON.stringify would throw an error otherwise. with your change of safeStringify, circular refs stay in the object, making the method basically the same as JSON.stringify. The correct fix of the method is to replace if (typeof v === 'object') with if (typeof v === 'object' && v !== null).

@jepiqueau
Copy link
Contributor

@chriswep your proposed fix is working Any progress status from @jcesaremobile

@emirtb
Copy link

emirtb commented Jul 20, 2021

same problem, two days lost ... and there is no way to solve it because it is the base framework

@jepiqueau
Copy link
Contributor

@emirtb I am sorry for this time lost but there is nothing I can do it is in the hands of the Ionic Capacitor team

@emirtb
Copy link

emirtb commented Jul 20, 2021

@jcesarmobile @chriswep seems that if you modify in assets/native-bridge.js the constant safeStringify by

const safeStringify = (value) => {
    const seen = new Set();
    return JSON.stringify(value, (_k, v) => {
        if (typeof v === 'object') {
            seen.add(v);
        }
        return v;
    });
};

it works , but i do not know what is the impact of doing that on the rest of the code and what the reason to have these lines

                if (seen.has(v)) {
                    return '...';
                }

Hope this will help

@jcesarmobile @chriswep seems that if you modify in assets/native-bridge.js the constant safeStringify by

const safeStringify = (value) => {
    const seen = new Set();
    return JSON.stringify(value, (_k, v) => {
        if (typeof v === 'object') {
            seen.add(v);
        }
        return v;
    });
};

it works , but i do not know what is the impact of doing that on the rest of the code and what the reason to have these lines

                if (seen.has(v)) {
                    return '...';
                }

Hope this will help

To modify this file as a workaround, where is it located? in the node_modules?

Thanks

@jepiqueau
Copy link
Contributor

@elylucas @thomasvidas @chriswep Seems it is always there in 3.1.2 despite the Changelog.md core: Modify safeStringify to allow multiple null values (#4853) (854539b). If i look in https://github.com/ionic-team/capacitor/blob/main/android/capacitor/src/main/assets/native-bridge.js it is still

        const safeStringify = (value) => {
            const seen = new Set();
            return JSON.stringify(value, (_k, v) => {
                if (seen.has(v)) {
                    return '...';
                }
                if (typeof v === 'object') {
                    seen.add(v);
                }
                return v;
            });
        };

and not the proposed solution by @thomasvidas

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 11, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants