Zero-length NSData parameter binds to SQL null value #73

Open
snej opened this Issue May 29, 2012 · 7 comments

Comments

Projects
None yet
2 participants
@snej

snej commented May 29, 2012

A zero-length NSData object passed as a parameter binding for a SQL statement results in a SQL 'null' value, not a zero-length blob as expected.

    // Test case: This assertion fails.
    NSAssert(([_fmdb boolForQuery: @"SELECT ? not null", [NSData data]]), @"Empty data should not be null");

This is significant because SQL 'null' values have odd behavior with respect to things like equality comparison. Additionally, my app schema has a blob column where a null value means "there is no data" as distinct from "the data is empty", but this distinction gets lost when the data is saved.

I'm not sure why this happens. The binding is done by the following lines in -[FMDatabase bindObject:toColumn: inStatement::

else if ([obj isKindOfClass:[NSData class]]) {
    sqlite3_bind_blob(pStmt, idx, [obj bytes], (int)[obj length], SQLITE_STATIC);
}

It may be that sqlite3_bind_blob binds a null value if the data pointer is NULL or the length is zero, but that seems like strange behavior since there is already a sqlite3_bind_null function.

@snej

This comment has been minimized.

Show comment
Hide comment
@snej

snej May 29, 2012

I'm working around this by substituting an empty NSString (@""). This results in a zero-length string value in the column, which when I fetch it with -dataForColumn: results in an empty NSData. (The actual data type stored will be string not blob, but I don't do anything that checks the actual stored data type, I just ask for it as a blob.)

snej commented May 29, 2012

I'm working around this by substituting an empty NSString (@""). This results in a zero-length string value in the column, which when I fetch it with -dataForColumn: results in an empty NSData. (The actual data type stored will be string not blob, but I don't do anything that checks the actual stored data type, I just ask for it as a blob.)

@ccgus

This comment has been minimized.

Show comment
Hide comment
@ccgus

ccgus May 29, 2012

Owner

It looks like [[NSData data] bytes] returns a pointer to 0x00.

In bindText (which sqlite3_bind_blob calls), there's a check to make sure the data (zData) != 0. I'm guessing this is what is going on- so the part that inserts the data is skipped over.

I suppose FMDatabase could insert an empty string in the case of an empty data. It would at least be consistent. Can you think of any reason this might suck?

Owner

ccgus commented May 29, 2012

It looks like [[NSData data] bytes] returns a pointer to 0x00.

In bindText (which sqlite3_bind_blob calls), there's a check to make sure the data (zData) != 0. I'm guessing this is what is going on- so the part that inserts the data is skipped over.

I suppose FMDatabase could insert an empty string in the case of an empty data. It would at least be consistent. Can you think of any reason this might suck?

@ccgus

This comment has been minimized.

Show comment
Hide comment
@ccgus

ccgus May 29, 2012

Owner

The fix is easy:

else if ([obj isKindOfClass:[NSData class]]) {
        if (![obj bytes]) {
            // it's an empty NSData object, aka [NSData data].
            // in cases like this, we want it to behave just like an empty string.
            sqlite3_bind_blob(pStmt, idx, "", 0, SQLITE_STATIC);
        }
        else {
            sqlite3_bind_blob(pStmt, idx, [obj bytes], (int)[obj length], SQLITE_STATIC);
        }
    }

I'll bring it up on the mailing list to see if anyone has potential problems.

Owner

ccgus commented May 29, 2012

The fix is easy:

else if ([obj isKindOfClass:[NSData class]]) {
        if (![obj bytes]) {
            // it's an empty NSData object, aka [NSData data].
            // in cases like this, we want it to behave just like an empty string.
            sqlite3_bind_blob(pStmt, idx, "", 0, SQLITE_STATIC);
        }
        else {
            sqlite3_bind_blob(pStmt, idx, [obj bytes], (int)[obj length], SQLITE_STATIC);
        }
    }

I'll bring it up on the mailing list to see if anyone has potential problems.

@snej

This comment has been minimized.

Show comment
Hide comment
@snej

snej May 29, 2012

Thanks for the quick reply!

Hm. A cleaner workaround might be to insert a blob with zero length but a (arbitrary) non-NULL pointer. I'll give that a try and report back.

snej commented May 29, 2012

Thanks for the quick reply!

Hm. A cleaner workaround might be to insert a blob with zero length but a (arbitrary) non-NULL pointer. I'll give that a try and report back.

@snej

This comment has been minimized.

Show comment
Hide comment
@snej

snej May 29, 2012

Ah. Your fix does exactly what I just suggested. :)

I don't see how this could cause trouble, because it just inserts a zero length blob, which is exactly what the caller would expect. The only regression in an app could be if it accidentally made use of the mapping to null; doesn't seem like something one would do on purpose.

snej commented May 29, 2012

Ah. Your fix does exactly what I just suggested. :)

I don't see how this could cause trouble, because it just inserts a zero length blob, which is exactly what the caller would expect. The only regression in an app could be if it accidentally made use of the mapping to null; doesn't seem like something one would do on purpose.

@ccgus

This comment has been minimized.

Show comment
Hide comment
@ccgus

ccgus May 29, 2012

Owner

And considering [NSMutableData data] behaves exactly the way you'd like- I don't think it's going to be a problem to make this change. If anything, it'll bring consistency.

Owner

ccgus commented May 29, 2012

And considering [NSMutableData data] behaves exactly the way you'd like- I don't think it's going to be a problem to make this change. If anything, it'll bring consistency.

@snej

This comment has been minimized.

Show comment
Hide comment
@snej

snej May 29, 2012

Here's a slightly faster/smaller fix. I've tested it and it works for me:

else if ([obj isKindOfClass:[NSData class]]) {
    const void* bytes = [obj bytes];
    if (!bytes) {
        // it's an empty NSData object, aka [NSData data].
        // Don't pass a NULL pointer, or sqlite will bind a SQL null instead of a blob.
        bytes = "";
    }
    sqlite3_bind_blob(pStmt, idx, bytes, (int)[obj length], SQLITE_STATIC);
}

snej commented May 29, 2012

Here's a slightly faster/smaller fix. I've tested it and it works for me:

else if ([obj isKindOfClass:[NSData class]]) {
    const void* bytes = [obj bytes];
    if (!bytes) {
        // it's an empty NSData object, aka [NSData data].
        // Don't pass a NULL pointer, or sqlite will bind a SQL null instead of a blob.
        bytes = "";
    }
    sqlite3_bind_blob(pStmt, idx, bytes, (int)[obj length], SQLITE_STATIC);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment