Commit Graph

4 Commits

Author SHA1 Message Date
Pau Espin 4474f35770 db_sms_test: Remove libdbi expected driver load errors
Newer versions of libdbi print to stderr unconditionally when trying to
load drivers from /usr/lib/dbd. This makes test output to change
depending on host/distro set up (installed modules).

Let's get those messages out to make it easier for people having tests
pass.

We swap stderr/stdout instead of mixing to avoud future possible race
conditions if both get content writen into them.

Change-Id: Iec78826d28435f464be22e81b3776a6ae8326d59
2019-06-05 17:01:06 +00:00
Pau Espin 17aa464b99 db_sms_test: Do not print exact memcmp result
man memcp doesn't define exact values for returned integer, it only
specifices a meaning for the sign of it.
So it happens that different versions/implementations actually return
different values when this test is run, making it fail.
Let's simply drop that info from logs since anyways it's not useful.

Change-Id: I771fb8f4fc56f337b16561d005ff1803a386d1c6
2019-06-04 11:18:39 +02:00
Vadim Yanitskiy 9d61db7f06 libmsc/db.c: fix storing SMS with empty TP-User-Data
Thanks to db_sms_test, it was discovered that storing an SMS with
empty TP-User-Data (TP-UDL=1) causes buffer overruns in libdbi
and it's SQLite3 driver (libdbdsqlite3):

  DDB NOTICE test_db_sms_store('Empty TP-UD'): ==7791== Invalid write of size 2
  ==7791==    at 0x857DC60: dbd_quote_binary (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
  ==7791==    by 0x5B2B321: dbi_conn_quote_binary_copy (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
  ==7791==    by 0x4073B1: db_sms_store (db.c:701)
  ==7791==    by 0x405BB5: test_db_sms_store (db_sms_test.c:310)
  ==7791==    by 0x405BB5: main (db_sms_test.c:546)
  ==7791==  Address 0x7ed1cf0 is 0 bytes after a block of size 0 alloc'd
  ==7791==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==7791==    by 0x857DC4B: dbd_quote_binary (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
  ==7791==    by 0x5B2B321: dbi_conn_quote_binary_copy (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
  ==7791==    by 0x4073B1: db_sms_store (db.c:701)
  ==7791==    by 0x405BB5: test_db_sms_store (db_sms_test.c:310)
  ==7791==    by 0x405BB5: main (db_sms_test.c:546)

  ...

  DDB NOTICE test_db_sms_get('Empty TP-UD'): ==8051== Invalid read of size 1
  ==8051==    at 0x5B30510: _dbd_decode_binary (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
  ==8051==    by 0x857D957: dbd_fetch_row (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
  ==8051==    by 0x5B2C86E: dbi_result_seek_row (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
  ==8051==    by 0x40828F: next_row (db.c:188)
  ==8051==    by 0x40828F: db_sms_get (db.c:805)
  ==8051==    by 0x406C29: test_db_sms_get (db_sms_test.c:390)
  ==8051==    by 0x405C14: main (db_sms_test.c:547)
  ==8051==  Address 0x8f74641 is 0 bytes after a block of size 1 alloc'd
  ==8051==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==8051==    by 0x5DBEB49: strdup (strdup.c:42)
  ==8051==    by 0x857D93C: dbd_fetch_row (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
  ==8051==    by 0x5B2C86E: dbi_result_seek_row (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
  ==8051==    by 0x40828F: next_row (db.c:188)
  ==8051==    by 0x40828F: db_sms_get (db.c:805)
  ==8051==    by 0x406C29: test_db_sms_get (db_sms_test.c:390)
  ==8051==    by 0x405C14: main (db_sms_test.c:547)
  ==8051==
  success, as expected
  DDB NOTICE verify_sms('Empty TP-UD'): user_data_len mismatch: E0 vs A3

Apparently, dbi_conn_quote_binary_copy() doesn't properly handle
zero-length input. Let's guard against this.

Observed with:

  - libdbi-dev 0.9.0-1
  - libdbd-sqlite3:amd64 0.9.0-2ubuntu2

Change-Id: If0b2bb557118c5f0e520a2e6c2816336f6028661
2019-06-03 17:28:44 +07:00
Vadim Yanitskiy e1e7247500 Introduce initial unit test for db_sms_* API
Since OsmoMSC has built-in SMSC, it needs to store the messages
somewhere. Currently we use libdbi and SQLite3 back-end for that.

For a long time, the db_sms_* API remained uncovered by unit tests.
This change aims to fix that, and does cover the following calls:

  - db_sms_store(),
  - db_sms_get(),

  - db_sms_get_next_unsent(),
  - db_sms_mark_delivered(),

  - db_sms_delete_sent_message_by_id(),
  - db_sms_delete_by_msisdn(),
  - db_sms_delete_oldest_expired_message().

Due to performance reasons, the test database is initialized in
RAM using the magic filename ':memory:'. This is a feature of
SQLite3 (and not libdbi), see:

  https://www.sqlite.org/inmemorydb.html

Of course, this unit test helped to discover some problems:

  1) Storing an SMS with empty TP-User-Data (TP-UDL=0) causes
     buffer overruns in both db_sms_store() and db_sms_get().

  2) TP-User-Data-Length is always being interpreted in octets,
     regardless of DCS (Data Coding Scheme). This results in
     storing garbage in the database if the default 7-bit
     encoding is used. Fortunately, the 'user_data' buffer
     in structure 'gsm_sms' is large emough, so we don't
     experience buffer overruns.

  3) db_sms_delete_oldest_expired_message() doesn't work
     as expected. Instead of removing the *oldest* expired
     message, it tries to remove the *newest* one.

The current test expectations do reflect these problems.
All of them will be fixed in the follow-up patches.

Change-Id: Id94ad35b6f78f839137db2e17010fbf9b40111a3
2019-06-03 17:28:35 +07:00