Skip to content

Add unescape to getAllType function #6804

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PTR-inc
Copy link
Contributor

@PTR-inc PTR-inc commented Feb 22, 2025

This is used in the recode encrypted fields function

Some db functions have the un/escape in the function, but not all. For some it is done in app after the get.

I would propose to move it to the db functions instead of having to not forget it at the application level. I haven't checked all functions, but the GetAll function also doesn't do the unescaping for you.

@si458 What do you think?

This is used in the reencode encrypted fields function
@PTR-inc
Copy link
Contributor Author

PTR-inc commented Feb 22, 2025

The recode function didn't work properly because of the missing unescape in the getalltype function. That triggered this.

@PTR-inc PTR-inc marked this pull request as draft February 23, 2025 08:42
@PTR-inc
Copy link
Contributor Author

PTR-inc commented Feb 23, 2025

@si458 I think a full check on the mongo/nedb un/escape db functions is necessary to do this properly.

@PTR-inc
Copy link
Contributor Author

PTR-inc commented Mar 1, 2025

@si458 Checked some of the functions used throughout the application for the different (un)escapes, here for example is the specific acebase db escapes, beside the nedb escapes. The only escape needed for mongo (v3.6 onward) is that a field must not start with a '$'.
Searching for 'unescape' in the source finds f.e. here and others, which, if forgotten in new functions, could lead to the bug fixed with this PR for the recode function. It would at least be safer to put all the (un)escaping in the db functions, I think.

Is it an idea to eliminate the need for escapes at all by generating the identifiers with something like crypto.randomUUID() or the UUID package?

That would remove a lot of overhead, room for mistakes and make it future-proof and more db agnostic.
What are your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant