Git Product home page Git Product logo

Comments (19)

jepiqueau avatar jepiqueau commented on June 12, 2024 1

@mmouterde I come back on this after futher tests on other platforms value like "O'Connor" works for web, iOS and Electron so it will be fixed for Android in next release.

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024 1

@mmouterde As already said the @capacitor-community/sqlite will not support the reserved words in the table's field name.
In 5.5.1-1, the following will work:

{
  database : "db-from-json492",
  version : 1,
  encrypted : false,
  mode : "full",
  tables :[
      {
          name: "company",
          schema: [
              {column:'id', value: 'CHARACTER(36) PRIMARY NOT NULL'},
              {column:'email', value:'TEXT UNIQUE NOT NULL'},
              {column:'name', value:'VARCHAR(25) NOT NULL'},
              {column:'age', value:'INT NOT NULL'},
              {column:'sql_deleted', value:'BOOLEAN DEFAULT 0 CHECK (sql_deleted IN (0, 1))'},
              {column:'last_modified', value:'INTEGER DEFAULT (strftime(\'%s\', \'now\'))'},
          ],
          values: [
              ["fe3c700c-9b21-11ee-b9d1-0242ac120002","[email protected]","O'Connors",55,0,1608216034],
              ["fe3c72d2-9b21-11ee-b9d1-0242ac120002","[email protected]",`L'awson has "nearly" something`,32,0,1608216034],
              ["fe3c7408-9b21-11ee-b9d1-0242ac120002",'[email protected]','Bush',44,0,1608216034],
        ]
      },
    ]
}

as you will notice

  • to store UUID in a field the field type is CHAR(36) or TEXT (this was already working in previous releases)
  • In a Json Object if you have a ' char you can define the value like this "O'Connor"
  • In a Json Object if you have multiple ' char and multiple " char the value must be defined as L'awson has "nearly" something

in an execute command:

const row: Array<Array<any>> = [[`W''hi"teley`,'Whiteley.com',30.2],[`J''o"ne"s`,"Jones.com",44]];
const twoUsers: string = `
INSERT INTO users (name,email,age) VALUES ('${row[0][0]}','${row[0][1]}',${row[0][2]});
INSERT INTO users (name,email,age) VALUES ('${row[1][0]}','${row[1][1]}',${row[1][2]});
`;
 const ret = await db.execute(twoUsers, true, true);

in a run command:

      const sql = `INSERT INTO users (name,email,age) VALUES (?,?,?);`;
      const val = [`J'ee"pq`,'[email protected]',45];
      const ret = await db.run(sql, val, false, 'no', true);

Voilà

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde you must use double quote '' to escape ' chars ie 'O''Connors'

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

Issue fixed in 5.5.0 but there is a regression for value containing " chars

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde as i already you have to give me a repro of your app wirh database scheme and the kind of datathat you are using so i can figure-toi right answer to your ovrall whiches. You cannot have both ' and " in the data or i must see how to do this and rewrite the code. Prior to do this, i want to see your app. You are the only one asking for this. This plugin is used by others for more thzn 4 years and nobody ask for this..

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024
  • Unfortunately my app is not open-source, I can't share directly my repo
  • Let me know how to provide a PR with failing unit test
  • However, you don't need a specific piece of code, even not a specific data... just having ' or " in any TEXT column value make the ImportFromJSON failed.

I'm also wondering if this plugin is made for my use case : With this community, this long life, I can understand why this kind of bugs is not detected of even tested. Here is some potential specificity that could differ from most project (?!) :

  • I build an offline first app, that need to sync data with a server
  • this app is collaborative and data could be changed by several users (server is the truth), I handle conflict on my side with business rules.
  • I'm using an API that return plain JSON to be provided to importFromJSON

My understanding seems to not fit with the user prerequisites.
In my understanding, capacitor sqlite plugin provides two layers of service : SQLite wrapper (that allow to send directly SQL to the DB) and a Data Layer (that provides import/Export JSON, sync Table... that generate SQL code).
As data layer user, I would expect that any SQL considerations is handled by the layer : whatever my data contains the data ends in the database. That implies : escaping reserved column name or reserved chars.

It's a pity, SQLite clients provide tools that avoid escaping characters and even wrap text as string, an not wrap integet. It's using preparedStatement (with questionmarks) and getting a values array. In the same way you did in Jeep-SQLite https://github.com/jepiqueau/jeep-sqlite/blob/17ccc58de5c2520b30fcc0816c6b56e8a1c39b08/src/utils/utils-importJson.ts#L438
I tried a PR to suggest code rewritings to benefit of this included feature but it require too many changes and I'm not confident with side effects.

From now, I could change my mind and consider to prepare my JSON values to be directly concatenated in a SQLStatement (no questionMark used).

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde i do not have a problem to rewrite the code if i understand your needs which is not the case right now.
I understand now that the following

  • The table field name if it use a SQLite keyword should be enclosed by double-quotes character ie for index the must be "index".
    What is not clear is
  • when you use preparedStatement with ? meanong that you have to provide an array of values , the value if it is a string must be enclosed with single-quote or double-quotes. So if you do it with single-quote you may have double-quotes characters inside the value and if you do it with double-quotes you can have single-quote characters inside the value. So i do not know how to do if you want both.
    So can you clarify

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde can you create a fake table schema and a fake set of data in a fake Json Object having all your needs and share it to me so i can test it

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde the only way to have both single-quote and double-quotes is

  • "O'Connor has ""nearly"" something"
  • 'O''Connor has "nearly" something'
    Both syntax are working
    Is it what you want ?
    I

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

Escape reserved key words in query as identifier

According to SQLite documentation, a word in double-quote is always considered as an identifier (even if match with a reserved word). So, as a lib, to safely handle column names, table names, we always have to wrap such identifier in double quote.
Each time we introspect schema to get column names, we have to make sure to wrap this identifier in double quote to be safe.

Escape single quote in data

Literal value in SQL should be wrapped in single quote. In case the literal value itself includes a single quote, we have to pay attention when we build the statement manually (concatenating strings) to avoid value interference on the request. SQL injection and security issues use this breach. The literal value should be wrapped in single quote AND all single quote in the value should be escaped too. Escaping single quote in value could be made by replacing the single quote by two single quotes (not double quote).
According to https://www.sqlite.org/lang_expr.html:

A string constant is formed by enclosing the string in single quotes ('). A single quote within the string can be encoded by putting two single quotes in a row - as in Pascal. C-style escapes using the backslash character are not supported because they are not standard SQL. 

Insert literal

When we build statement manually, we have to know the type of each literal. Integer could be concanenated directly while String should be wrapped in single quote and single quote in the string escaped.
Relative to the issue #495, DELETE table WHERE ID IN (1,2,3) works ids are integer so "IN ("+id1+","+id2+","id3+")" works. But in my case, id are UUID as TEXT column. so id has to be wrapped as string literals.

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

From these theorical points, I would say (opinion) that building dynamic SQL statement using StringBuilder or concatenating string is a wrong pattern.

I suggest to use the QueryBuilder provided by Android SQLite API, or use insert(), update() delete() methods and provide them a SQL Statement built with question marks as first argument, and value array as second argument.
This way all stuff about escaping indentifiers or single quote seen my last comment would (not tested in sqlite but experienced with all SQL client I met until now) be handled automatically.

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde can you answer to what i ask

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

Sure !

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

I think the only point I did not replied is the fake example right ? (work in progress)

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

Here is a all-tricks-in-one JSON.

I have not tried schema update from JSON yet, I usedy Database Incremental Upgrade where my index column is wrapped in double quote by myself as I write plain SQL on my own.

{
  "database": "db-from-json",
  "version": 1,
  "encrypted": false,
  "mode": "partial",
  "tables": [
    {
      "name": "users",
      "schema": [
        { "column": "id", "value": "INTEGER PRIMARY KEY NOT NULL" },
        { "column": "index", "value": "INTEGER" }, /* index is a reserved word. should be escaped by double quote in SQL statement*/
        { "column": "email", "value": "TEXT UNIQUE NOT NULL" },
        { "column": "name", "value": "TEXT" },
        { "column": "age", "value": "INTEGER" },
        { "column":"sql_deleted", "value": "BOOLEAN DEFAULT 0 CHECK (sql_deleted IN (0, 1))"},
        { "column":"last_modified", "value": "INTEGER DEFAULT (strftime('%s', 'now'))"}
      ],
      "values": [
        [
          "066c8b42-fa09-45db-8b18-5624323342cc", /* uuid as string, means not usual integer auto inscrement, causes #495 */
          0,
          "Devil User with \"trick\" in data", /* double quote in string, make 5.5.0 fix failed*/
          "O'Connor", /* single quote in string, make concatenated sql statement fail, causes #492 */
          22,
          0,
          1601972413
        ]
      ]
    }
  ]
}

from sqlite.

jepiqueau avatar jepiqueau commented on June 12, 2024

@mmouterde thaks it is all what i need from the begining. Next time please give a fake example asap when describing an issue it will avoid me to waste my time. It is not as easy as typescript or a Json object needs to have the \ symbol before the single-quote and double-quotes where SQL statement needs to double the single-quote or double-quote. For index even if you create it with "index" when you create the table when you want to get the column names it is retrieve as index and not "index" which is making the prepareSQL to failed meaning that we have to check in the array of column name if the name is a reserved keyword. This will slow down the importFromJson process a lot so i still think that is better to avoid to use réservez keywords in table identifier name.this could be done by having an entry parameter set to false by default and if set to true by the user the check is done. Quite a work

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

Ok, sorry for the wasted time.

Note that when a maintainer formalizes automated tests (at least one) reporters could know how to give you a test case (with data) that fit their use cases or their needs.

As you can see with my reported issue on jeet-sqlite, I usually start by formalizing a PR that illustrates my use case. I did not find in this repo how to provide such code or data, and ask for it in my third message.

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024

About the performance point, as native Java client should already handle escaping ' " and reserved word, we would rather let him work, that means no extra cost and a better robustness.

I agree, it's an impacting rework

from sqlite.

mmouterde avatar mmouterde commented on June 12, 2024
  • thanks for your reactivity !
  • Ok for excluding reserved word. your choice.
  • I confirm that single quote issue is closed

from sqlite.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.