Several issues with db client about julea HOT 5 CLOSED

@Qualenritter Could you please take a look?

from julea.

I found a few issues when writing tests for the db client:

1. j_db_schema_new does not properly check the namespace and name given. For instance, if they contain a -, errors will occur later on (see https://github.com/wr-hamburg/julea/blob/master/test/db/db.c#L68). This probably also applies to other parts of the db client.

Ok, I could write a regex, to whitelist the allowed namespaces and names.
For example ([a-zA-Z][a-zA-Z0-9_]*) should remove all errors of this kind.
Maybe this could be defined globally, since other backends for example "j_distributed_object_new" have the same issue too.

2. Within the db client, errors seem to be overwritten somewhere (caused by 1.):

GError set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
The overwriting error message was: sql prepare failed error was <DROP TABLE IF EXISTS test-ns_test_schema_0> 'near "-": syntax error'

This should not happen, I'll try to reproduce/fix this.

3. Reference counting seems to be broken. If schemas are created in a loop with g_autoptr but the batch is executed outside of the loop, the address sanitizer complains about heap-use-after-free (see https://github.com/wr-hamburg/julea/blob/master/test/db/db.c#L110).

Objects, which are added to the network component are currently not reference counted.
The reason is, that every object is splitted into its variables, and there is no reference counting for "char*" or bson_t or GError.
I'll try to find a "simple" solution for this.

4. https://travis-ci.org/wr-hamburg/julea/jobs/600321116

from julea.

Ok, I could write a regex, to whitelist the allowed namespaces and names.
For example ([a-zA-Z][a-zA-Z0-9_]*) should remove all errors of this kind.
Maybe this could be defined globally, since other backends for example "j_distributed_object_new" have the same issue too.

Apparently, SQLite supports these characters but they have to be quoted/bracketed: https://stackoverflow.com/questions/3694276/what-are-valid-table-names-in-sqlite

This should not happen, I'll try to reproduce/fix this.

Thanks!

Objects, which are added to the network component are currently not reference counted.
The reason is, that every object is splitted into its variables, and there is no reference counting for "char*" or bson_t or GError.
I'll try to find a "simple" solution for this.

It might be easiest to just reference the objects in their corresponding JBackendOperation (which will probably also need an unref_func member to not complicate the free function).

from julea.

Ok, I could write a regex, to whitelist the allowed namespaces and names.
For example ([a-zA-Z][a-zA-Z0-9_]*) should remove all errors of this kind.
Maybe this could be defined globally, since other backends for example "j_distributed_object_new" have the same issue too.

Apparently, SQLite supports these characters but they have to be quoted/bracketed: https://stackoverflow.com/questions/3694276/what-are-valid-table-names-in-sqlite

To prevent any kind of sql-injection, a whitelist to only letters and numbers is safe.
Is there a specific reason to allow '-' ?
Otherwise a '_' could be used in the middle of the name to have the same effect.
The interface was designed to allow any backend - not only SQLite - therefore we may not allow every possible special character because this may produce errors later.

This should not happen, I'll try to reproduce/fix this.

Thanks!

Objects, which are added to the network component are currently not reference counted.
The reason is, that every object is splitted into its variables, and there is no reference counting for "char*" or bson_t or GError.
I'll try to find a "simple" solution for this.

It might be easiest to just reference the objects in their corresponding JBackendOperation (which will probably also need an unref_func member to not complicate the free function).

I had a similar idea, but implementation will show if it works.

I think these aspects need to be considered:

1. The object itself must be linked within the JBackendOperation
2. ref count issues occur only on the client side - there is no object on the server -> no new code for the server.
3. The (un)ref_func needs to be linked
4. GError is supplied by the user. GError is NOT related to any specific Julea-DB-Object. If the user supplies the GError, the user must make sure, that GError is valid as long as it might be used - in this case the livetime of JBatch. If GError is freed within the loop, this is a programming error in the user code, not an error of Julia-DB. If the user don't want to check GError, the user must supply a NULL-ptr instead. Otherwise GError must be integrated into (and be owned by) JBatch.

from julea.

The stack overflow is still happening: https://travis-ci.org/wr-hamburg/julea/jobs/614492687

Any progress on this?

from julea.