mirror of
https://github.com/Rolands-Laucis/Socio.git
synced 2026-05-15 06:05:53 -06:00
[GH-ISSUE #16] Interpolation in queries & strange behaviour with ' in place of " #4
Labels
No labels
help wanted
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/Socio#4
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @KaruroChori on GitHub (Apr 6, 2023).
Original GitHub issue: https://github.com/Rolands-Laucis/Socio/issues/16
At the moment, it seems like queries in this form are failing:
The interpolation of a table/view name is particularly useful when targetting custom views based on a source table, to split the update triggers and avoid propagating changes globally.
Also it seems like writing 'Bob' or "Bob" is not the same, as the first will fail, resulting in:
Error: the cipher text does not contain exactly 3 space seperated parts, therefor is invalid. [#cipher-text-invalid-format]
@Rolands-Laucis commented on GitHub (Apr 6, 2023):
I dont understand what you mean with the first paragraph, but i think this isnt a Socio related issue, as Socio doesnt handle constructing queries or modifying them. Just parsing them in some cases.
If you are using Sequelize's raw query function, then see this documentation about how the replacements work. In the demos, the params object givent to SocioClient.Query('...', params) is literally given to sequelize.query('...', {replacements:params}). Seems like sequelize seems to have trouble parsing that replacement syntax.
The given query code snippet produces
SequelizeDatabaseError: SQLITE_ERROR: near ":ch": syntax errorfor me in the basic demo env. Perhaps a replacement near a table name is invalid?If :ch is used by itself without the 'users' part, then it gives
SequelizeDatabaseError: SQLITE_ERROR: no such table: 001a2ff, which is logically correct.And if
users :chis used with a space:SequelizeDatabaseError: SQLITE_ERROR: near "'001a2ff'": syntax error, which is again correct.However, reading the documentation thoroughly myself, I realize that an array can also be passed instead of an object, so i will be fixing that expected type in Socio some time later.
As for the last paragraph - i cannot reproduce. Both 'Bob' and "Bob" should be work identically, since it will be JSON.stringified through the network and arrive to the backend as "Bob" in both cases, as per the JSON spec. Still, both are afaik identical notations for a string literal in JS, so it wouldnt matter even if the quotes were different.
In addition, the [#cipher-text-invalid-format] error would only pop up when you are using SocioSecurity and the queries are expected to be received encrypted. SocioServer has also flags for disabling decipher completely, just for SQL or just for props (as they can also be encrypted).
@KaruroChori commented on GitHub (Apr 6, 2023):
As for the second issue, I am just basing it on your svelte example:
Line 134 of +page.svelte
or
The first one is working as expected, the second one results in the error I described earlier. Can you replicate it this way?
It makes no sense to me as well.
I just assumed some of the automagical regex of the vite plugin were not working there, but I failed to identify the source of the problem.
I can upload the affected code in a fork if it helps.
@KaruroChori commented on GitHub (Apr 6, 2023):
As for the first point, thank you for the reply.
It seems so. It is probably a limitation in the interpolation of sequelize.
But the second option is as good and it would work, so I will stick to that :).
@Rolands-Laucis commented on GitHub (Apr 6, 2023):
Okay, so i found the actual issue, since ofc the symptoms dont make any sense.
The full-stack demo index route +page.svelte file contains my js comment line, that includes a single ' apostrophe in the text
which means the entire document now has an uneven count of ' symbols. Then the SocioSecurity parses the file, and all the single quotes from then on are offset/inverted. Which means that the SQL string is not being found and encrypted. Ergo, the decipher error. Removing the comment or just that 1 ' fixes the issue.
Thank you for bringing this to my attention, as i had assumed with my regex that files would always only contain even counts of quote symbols, since language syntaxes require that. But i had not considered other text that might be in files. This also means that any apostrophe in html etc. would cause the same issue from that point on in the file.
I will think about how to fix this. Perhaps you have some ideas? Seems like a tricky problem.
On an unrelated note, you keep using the word "Interpolation", but i now think you actually mean "implementation" :)
@KaruroChori commented on GitHub (Apr 6, 2023):
I will check over the weekend if I can find any reasonable solution.
I really mean string interpolation: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#string_interpolation
It is a semantic extension of the usual concept of interpolation in math/statistics.
The operation of interpolating substrings into a pattern is often referred as such.
@KaruroChori commented on GitHub (Apr 7, 2023):
The easiest way I found is to use prettier to ignore comments:
https://stackoverflow.com/questions/28963501/strip-comments-from-javascript-file
Also, it detects strings directly, so there is no need to use additional regex for that. You can work on the AST of the original file and integrate any side-effect desired.
Other solutions based on regex you might find like this one here: https://stackoverflow.com/questions/37051797/remove-comments-from-string-with-javascript-using-javascript
will not work, because comments inside strings are also detected.
@Rolands-Laucis commented on GitHub (Apr 7, 2023):
Using an engine like prettier for such a problem seems overkill. And the AST parsing idea would make the parsing less general, i.e. only certain files would be parsable by the parser, and only those then usable by Socios Vite plugin. I'd like to stick to regex, as then the file type and content is irrelevant, so long as it is plaintext.
I've come up with a longer and more complex regex pattern, by combining the old 2 and fixing a problem.
^idk how to escape githubs markdown, so this will look wrong.
Regex101 demo. For JS this seems to work on the full-stack demo +page.svelte whole file. newlines and tabs etc. Matches only the socio strings and captures the semantic groups correctly. I will think about this for a while longer, and if you also dont find anything wrong with this, then i will eventually replace the existing parsing logic to just use this instead. Should also give a performance boost for the vite plugin as well i guess.
Turns out Backreferences dont work in
[^\1]to match anything except the first capture, i.e. the starting quote symbol. Which i had in the old regex. They only mention this at the end of the page :D greatSo instead i use the (?:(?!\1)(?:.|\n))*? negative lookahead (?!\1)
Also i was not aware of the "string interpolation" term 👌
@KaruroChori commented on GitHub (Apr 7, 2023):
It seems fine.
I was able to come up with few examples where it fails or matches more than what would be reasonable, but they are very much intentional & artificial.
To be honest I think an approach based on tagged templates would be better, as it is semantically supported by js/ts.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#tagged_templates
Basically, writing socio queries would be like
For reference, this concept applied to sequelize: https://dev.to/newbie012/please-dont-manually-parameterize-your-sql-queries-3m7k
@Rolands-Laucis commented on GitHub (Apr 7, 2023):
That is actually brilliant. Where were you like 3 months ago :D Before the SocioSecurity class, i had been thinking about ways to have a marker somewhere at the front of the string rather than at the end, like putting a $ or # , but that would be fine in either JS or SQL syntax.
I didnt know JS had such a tagged template feature. That is really powerful. All these years and JS can still find ways to make me love it more :)
So i guess, this will be used instead. The socio template tag will just be a dummy function. The regex will be much simpler to just find strings with that template tag and replace with encrypted string that doesnt even need the tag anymore. And perhaps i can create an actual SQL formatter function on the backend for devs that use sequelize.
The drawbacks i see here are that socio strings must always be template string literals. And they need the tag, which has to be imported from the lib or your own dummy function defined somewhere, just so IDE's dont complain. Also then the auth and perm markers will be handled slightly different. Since having an ending --socio-auth would be repetitive. There are a few different ways to handle them now tho, so thats fine.