Skip to content
This repository has been archived by the owner on Jul 25, 2020. It is now read-only.

Cursor::get_text() is not safe #92

Closed
dckc opened this issue Jul 26, 2014 · 0 comments · Fixed by #94
Closed

Cursor::get_text() is not safe #92

dckc opened this issue Jul 26, 2014 · 0 comments · Fixed by #94

Comments

@dckc
Copy link
Contributor

dckc commented Jul 26, 2014

sqlite3_column_text() can return NULL and get_text() doesn't handle it. For example, these tests will result in a SEGV (status=11):

    #[test]
    fn get_text_without_step() {
        let db = checked_open();
        let c = checked_prepare(&db, "select 1 + 1");
        assert_eq!(c.get_text(0).as_slice(), "");
    }

    #[test]
    fn get_text_on_bogus_col() {
        let db = checked_open();
        let c = checked_prepare(&db, "select 1 + 1");
        c.step();
        assert_eq!(c.get_text(1).as_slice(), "");
    }
lifthrasiir added a commit to lifthrasiir/rustsqlite that referenced this issue Jul 26, 2014
they used to return a direct `String` or `Vec<u8>`, which may fail
when it should return `NULL`. other functions do not have this issue
since the automatic conversion is fully defined over INTEGER and
FLOAT internal types.

this commit changes them to return an optional `&str` or `&[u8]`.
this correctly handles `NULL`, and moreover, reduces the redundant
allocations when the buffer doesn't need to be kept. this also
makes `Cursor::get_bytes` fully redundant, so it has been removed.
lifthrasiir added a commit to lifthrasiir/rustsqlite that referenced this issue Jul 26, 2014
they used to return a direct `String` or `Vec<u8>`, which may fail
when it should return `NULL`. other functions do not have this issue
since the automatic conversion is fully defined over INTEGER and
FLOAT internal types.

this commit changes them to return an optional `&str` or `&[u8]`.
this correctly handles `NULL`, and moreover, reduces the redundant
allocations when the buffer doesn't need to be kept. this also
makes `Cursor::get_bytes` fully redundant, so it has been removed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant