Re: Unreliable next_msg_id (was Re: [PATCH 2/2] Added SQLite history plugin)
Hi Niko, Hi! At first look it seems that ofono stores permanently the next_msg_id only when the sms_remove functions is called, and that's does not happen when ofono quits in a not clean way. During our tests, users reported bad behaivour of the sqlite history plugins and error messages about sqlite constraints violation, after a rude power off of the device (battery drained, forced shutdown with long press power button, and so on). So the immediate fix is to set sync the SMS settings whenever the next_msg_id is updated. That's becouse ofono on the next start reads an old next_msg_id from the permanent storage, so when sending/receiving a new message the history plugin fails to insert a new row as one with the same msg_id was already inserted in the prev session. Any suggestion to fix that? The proper fix is to generate a SHA1 or MD5 hash over the contents, local reception/send time, remote reception / send time, destination / originator of the message. Finally, we'd like to use some panic function in ofono, that should power down all modems and warn clients when critical conditions happen. Is that possible? Send HUP to the daemon? Or do you want oFono to keep running? Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Unreliable next_msg_id (was Re: [PATCH 2/2] Added SQLite history plugin)
On Thu, Apr 15, 2010 at 8:35 PM, Denis Kenzior denk...@gmail.com wrote: [...] During our tests, users reported bad behaivour of the sqlite history plugins and error messages about sqlite constraints violation, after a rude power off of the device (battery drained, forced shutdown with long press power button, and so on). So the immediate fix is to set sync the SMS settings whenever the next_msg_id is updated. Yes, this for sure will help a lot. That's becouse ofono on the next start reads an old next_msg_id from the permanent storage, so when sending/receiving a new message the history plugin fails to insert a new row as one with the same msg_id was already inserted in the prev session. Any suggestion to fix that? The proper fix is to generate a SHA1 or MD5 hash over the contents, local reception/send time, remote reception / send time, destination / originator of the message. Dario suggested that to me too, the only problem I see is that an hash signature does not provide operator that may help in some sync scenarios, but this may be fixed in several ways (for example using an additional autoincrement serial column that may expose to applications an unique, ordered and reliable id without exposing the ofono internal one). Will check if all history callbacks lets us rehash the messages properly. A shot in the dark: what's about delegating the id generation to the history plugin (if present and capable)? That should fix the above problems, avoid hash generation and provide a possible base to reuse the id while resubmitting failed/pending messages providing the right id to ofono, avoiding the introduction of race conditions or complex implementations. Anyway this is a delicate and upcoming use case, we just created our git tracking to experiment some solutions, when ready will submit patches again. Finally, we'd like to use some panic function in ofono, that should power down all modems and warn clients when critical conditions happen. Is that possible? Send HUP to the daemon? Or do you want oFono to keep running? I mean the best way to shutdown modems and ofono inside the code (in the history for example if an sql statement fails, or if it cannot save id to the permanent storage, big dbus troubles or other similiar cases). We just discovered our users may become crazy if they lose a single sms/call :) Regards Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Unreliable next_msg_id (was Re: [PATCH 2/2] Added SQLite history plugin)
Hi Niko, The proper fix is to generate a SHA1 or MD5 hash over the contents, local reception/send time, remote reception / send time, destination / originator of the message. Dario suggested that to me too, the only problem I see is that an hash signature does not provide operator that may help in some sync scenarios, but this may be fixed in several ways (for example using an additional autoincrement serial column that may expose to applications an unique, ordered and reliable id without exposing the ofono internal one). I suggest using a sync timestamp or some other way to accomplish this. Will check if all history callbacks lets us rehash the messages properly. A shot in the dark: what's about delegating the id generation to the history plugin (if present and capable)? That should fix the above problems, avoid hash generation and provide a possible base to reuse the id while resubmitting failed/pending messages providing the right id to ofono, avoiding the introduction of race conditions or complex implementations. oFono will keep pending messages around across reboots, that is an enhancement we're already working on. Resubmitting of failed messages should result in a new ID anyway. And I honestly don't think N implementations of hashing SMS messages in each history plugin is a good idea. Let oFono do it so we get it right for everyone. Finally, we'd like to use some panic function in ofono, that should power down all modems and warn clients when critical conditions happen. Is that possible? Send HUP to the daemon? Or do you want oFono to keep running? I mean the best way to shutdown modems and ofono inside the code (in the history for example if an sql statement fails, or if it cannot save id to the permanent storage, big dbus troubles or other similiar cases). Yikes. The answer is Please don't even think about it :) Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Denis,
Hi Dario,
+#define SQL_HISTORY_DB_PATH STORAGEDIR /ofono_history.sqlite
+#define SQL_HISTORY_DB_SQL STORAGEDIR /oFono_History_DB.sql
So I have my concerns about storing this in /. This should be in a proper
directory, like /var/lib/ofono or something like that.
STORAGEDIR is a macro expanded by Autoconf to ${storagedir} (please see
rows 172-178 of configure.ac) which is normally /var/lib/ofono so the DB
and the scripts are already stored inside that dir.
So remember that the history plugin is instantiated for each and every modem,
so you will be sqlite_open/sqlite_closing the db in each instance. Not really
sure if this is what you want, or whether you want a reference-counted
database connection here.
Database connection is handled by sqlite3_* API in a transparent manner
so you could have different open connections at the same time without
any problem, each SQL statement is transactional so the DB is every time
consistent, but if you prefer I can rethink of it (maybe a DB for every
modem could be a possibility but this would overload the plugin because
we have to open and close the DB in every callback given that we have to
filter the files access by modem).
Another thing to consider is that you might want to store the IMSI of the
modem along with the history information for every call / sms event. That way
when a SIM card is changed, the user can be shown a different set of call / sms
history.
Thank you for this suggestion, didn't think about this :) I have one
question: what's the best way to get that info?
I see only one function to get the imsi information: const char
*ofono_sim_get_imsi(struct ofono_sim *sim) which needs a struct
ofono_sim pointer, but I don't have it in history as it's a member of
the struct ofono_modem which is blind for me. Can you please point me at
the function to achieve this?
Finally, you might want to set limits on the number of entries in the
database, and expire them as the limit is reached. Otherwise you'd need to
expire them periodically from e.g. cron, but would need to release control of
the database during that time.
Well this is a more complex topic because needs to point at the purpose
of the history plugins (in general); I thought at the plugin as a simple
event logger in which an event could be a call or a SMS; the
question is: must the plugin handle all the logic and possibly interface
with the applications (for example using DBus methods/signals) or is it
only a mere storage daemon letting applications handle the informations?
Talking about this with one of my team members (Nicola.mfb, which should
explain better than me applications point of view in a next email) we
arrived at the conclusion that the history plugin should be able to talk
directly to the applications (tipically sending DBus signals and
exporting methods) to mask the storage type of the history data.
Also in this case there could be some possible issues to handle: who is
aware of the msg_id? Only the plugin or even the application? In the
former case must the application provide to the plugin its own msg_id to
recover SMS data?
Another question is: what if we had different history plugins at the
same time, like an EDS one, a plain text one, a Berkley DB one and an
SQLite one? There should be only one plugin active per ofonod instance
or per modem? In case different plugins are active at the same time, the
history DBus interface is global for all the plugins (i.e. a method to
delete an incoming sms from modem /modem0 with msg_id = 123 must delete
it on all backends?) or each plugin has its own?
Thank you very much in advance, best regards,
Dario
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Tue, Apr 6, 2010 at 7:56 PM, Denis Kenzior denk...@gmail.com wrote: [...] Hi Denis, Actually our apps listen for the IncomingMessage signal, and store the SMS in an internal database. At some point by design, a restart, a crash or other reasons, it may happen that apps are down while ofono and the modem are up, in that case we lose incoming messages. To solve this we tought to use the history, so, for example, clients at startup query the plugin, processes lost messages and further continue listening for the IncomingMessage signal. We have a problem here, as the IncomingMessage lacks for the sms id property, we generate an internal id that is out of sync respect of ther history id, so at the next app restart we cannot determine what messages were lost. The second problem is that we may use several clients at the same time, thinks for a phone app, and a desktop widget showing the number of incoming/unread etc. messages. The two clients needs to access the same informations in a consistent way. For that reason, and due the necessity to abstract the db access with a dbus api, we thought the natural way is to use the history plugin as a permanent storage plugin too, improving the dbus interface to have clients cooperating in a consistent manner, and avoiding duplicate sms/calls data in different places. The phone app may request to delete a message, and a HistoryMessageDeleted signal should be emitted, and so on. Here we may have an HistoryIncomingMessage signal that provides the id field too and fix the first issue (if you do not prefer to add the id field in the IncomingMessage signal directly). The plugin may be used to store further metadata providing signals to inform of their changes, that my be useful in all clients (if a message was read in the phone app, the desktop widget should take care of that and decrease the showed number of new messages). Finally apps may decide with the right dbus calls if sms listing has to be filtered by sim or not, or when and how expire the history. But thats impacts a lot on the general perspective of the history plugin usage and further developing. Infact if a complete dbus api is going to be defined, it would be nice to share it for all history plugins, adding for example an org.ofono.HistoryManager interface to the modem, while if the history plugin has to be used in a different manner, or ofono does not care of that by design, we have to implement the dbus interface in the sqlite plugin directly, forcing our apps to not be generally oFono compliants (at least when not using the sqlite plugin). So we are asking to you what's are your plans and how may we proceed. Thanks for your support! Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/2] Added SQLite history plugin
Hi Denis, Actually our apps listen for the IncomingMessage signal, and store the SMS in an internal database. At some point by design, a restart, a crash or other reasons, it may happen that apps are down while ofono and the modem are up, in that case we lose incoming messages. A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE. Cheers, Waldo ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Waldo, Hi Denis, Actually our apps listen for the IncomingMessage signal, and store the SMS in an internal database. At some point by design, a restart, a crash or other reasons, it may happen that apps are down while ofono and the modem are up, in that case we lose incoming messages. A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE. If oFono crashes/runs out of battery here the driver is gone along with oFono. Notifying it of anything is not going to help ;) Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Wed, Apr 7, 2010 at 7:33 PM, Denis Kenzior denk...@gmail.com wrote: [...] A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE. If oFono crashes/runs out of battery here the driver is gone along with oFono. Notifying it of anything is not going to help ;) Yes, anyway I'm thinking about the case when the storage system is full, or an other error occours while trying to store the message. In this case the driver should notify the network with a NACK, while a signal should notify the user that the storage is full or there is another problem. Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Wed, Apr 7, 2010 at 7:01 PM, Denis Kenzior denk...@gmail.com wrote: [...] Database connection is handled by sqlite3_* API in a transparent manner so you could have different open connections at the same time without any problem, each SQL statement is transactional so the DB is every time consistent, but if you prefer I can rethink of it (maybe a DB for every modem could be a possibility but this would overload the plugin because we have to open and close the DB in every callback given that we have to filter the files access by modem). My concern here was that multiple open connections to the same database might require locking or other overhead that could be avoided with a single shared connection. It might be there is no overhead and your original approach is fine. What's about a dbfile for each modem? we'll have one connection per database, and may avoid the modem column on all the tables, saving storage space. [...] We're working on improving the SMS capabilities, and might eventually expose the message id. However, the message id really has no meaning, so this is not something we really want to expose. I'd like to keep it private between the history plugin and the core. Are you sure you really require it to be exposed, or can you do without it? We prefer to have the id to easily sync with the history plugin, as said in previous mail, if you prefer to not expose the id on IncomingMessage, we may emit an HistoryIncomingMessage signal with the id. Regards Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Niko, On Wed, Apr 7, 2010 at 7:33 PM, Denis Kenzior denk...@gmail.com wrote: [...] A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE. If oFono crashes/runs out of battery here the driver is gone along with oFono. Notifying it of anything is not going to help ;) Yes, anyway I'm thinking about the case when the storage system is full, or an other error occours while trying to store the message. In this case the driver should notify the network with a NACK, while a signal should notify the user that the storage is full or there is another problem. If your storage is full then you have other problems. Adding nack/ack capability would increate the complexity of the driver/core about 10 fold, so this is not something we're willing to consider right now. If this is an issue, then the user should be notified well in advance to rectify the situation or face possible data loss. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/2] Added SQLite history plugin
Hi Waldo, Hi Denis, Actually our apps listen for the IncomingMessage signal, and store the SMS in an internal database. At some point by design, a restart, a crash or other reasons, it may happen that apps are down while ofono and the modem are up, in that case we lose incoming messages. A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE. If oFono crashes/runs out of battery here the driver is gone along with oFono. Notifying it of anything is not going to help ;) Currently the modem driver acknowledges the PDU right away. If there was a notification back to the modem driver, the PDU acknowledgement could be delayed till that point in time. If something bad happens in between, either the modem or the network can hold on to the PDU and redeliver once oFono is up and running again. Looking at at_cmt_notify() the current implementation might have that affect already, as AT+CNMA is only send after calling ofono_sms_deliver_notify(). So as long as ofono_sms_deliver_notify() is handled synchronously and ensures that the PDU is stored one way or another before returning, it will work in case of a crash. (Would like to see assumptions like that called out in the code) That said, if your storage has issues (disk full?) there is currently no way to let the modem driver know that delivery was unsuccessful. Cheers, Waldo ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Niko, On Wed, Apr 7, 2010 at 7:01 PM, Denis Kenzior denk...@gmail.com wrote: [...] Database connection is handled by sqlite3_* API in a transparent manner so you could have different open connections at the same time without any problem, each SQL statement is transactional so the DB is every time consistent, but if you prefer I can rethink of it (maybe a DB for every modem could be a possibility but this would overload the plugin because we have to open and close the DB in every callback given that we have to filter the files access by modem). My concern here was that multiple open connections to the same database might require locking or other overhead that could be avoided with a single shared connection. It might be there is no overhead and your original approach is fine. What's about a dbfile for each modem? we'll have one connection per database, and may avoid the modem column on all the tables, saving storage space. This is possible, or you can do 1 db for each IMSI. Do note that some drivers do not have IMSIs (e.g. making calls via HFP driver). So it is up to the implementation to decide whether to log those calls or not. [...] We're working on improving the SMS capabilities, and might eventually expose the message id. However, the message id really has no meaning, so this is not something we really want to expose. I'd like to keep it private between the history plugin and the core. Are you sure you really require it to be exposed, or can you do without it? We prefer to have the id to easily sync with the history plugin, as said in previous mail, if you prefer to not expose the id on IncomingMessage, we may emit an HistoryIncomingMessage signal with the id. Your plugin is free to do anything it wants. The real question is whether you truly need this id or you can simply update the application state based on the IncomingMessage signal only. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Wed, Apr 7, 2010 at 7:55 PM, Denis Kenzior denk...@gmail.com wrote: [...] Yes, anyway I'm thinking about the case when the storage system is full, or an other error occours while trying to store the message. In this case the driver should notify the network with a NACK, while a signal should notify the user that the storage is full or there is another problem. If your storage is full then you have other problems. Adding nack/ack capability would increate the complexity of the driver/core about 10 fold, so this is not something we're willing to consider right now. If this is an issue, then the user should be notified well in advance to rectify the situation or face possible data loss. That's not always possibile, for example in the case of filesystem/block device faults. I have a real case here, on the freerunner sometimes using the WiFi will broke the access to the SD media card, I know it's not a perfect device but similiar problems may happen on different hardware too. Anyway I was aware of such complexity, my old phones nacks when the sim/me/fs storage is full, hoping that will be addressed in some future. For now we will notify the user in advance and power down the modem. Thanks and regards Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Niko, On Wed, Apr 7, 2010 at 7:55 PM, Denis Kenzior denk...@gmail.com wrote: [...] Yes, anyway I'm thinking about the case when the storage system is full, or an other error occours while trying to store the message. In this case the driver should notify the network with a NACK, while a signal should notify the user that the storage is full or there is another problem. If your storage is full then you have other problems. Adding nack/ack capability would increate the complexity of the driver/core about 10 fold, so this is not something we're willing to consider right now. If this is an issue, then the user should be notified well in advance to rectify the situation or face possible data loss. That's not always possibile, for example in the case of filesystem/block device faults. I have a real case here, on the freerunner sometimes using the WiFi will broke the access to the SD media card, I know it's not a perfect device but similiar problems may happen on different hardware too. Anyway I was aware of such complexity, my old phones nacks when the sim/me/fs storage is full, hoping that will be addressed in some future. This is certainly possible to add, however the priority is rather low because only truly bizarre conditions result in loss of data. Namely disk full and physical device corruption. In both cases you have other more important information than SMS that will be lost. If this is important to you, you can always submit a patch :) Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Waldo, Currently the modem driver acknowledges the PDU right away. If there was a notification back to the modem driver, the PDU acknowledgement could be delayed till that point in time. If something bad happens in between, either the modem or the network can hold on to the PDU and redeliver once oFono is up and running again. Looking at at_cmt_notify() the current implementation might have that affect already, as AT+CNMA is only send after calling ofono_sms_deliver_notify(). So as long as ofono_sms_deliver_notify() is handled synchronously and ensures that the PDU is stored one way or another before returning, it will work in case of a crash. (Would like to see assumptions like that called out in the code) That said, if your storage has issues (disk full?) there is currently no way to let the modem driver know that delivery was unsuccessful. You described the current implementation perfectly. The driver can assume that the fragment has been stored after the call to sms_deliver_notify has returned. For its part, the history plugin should ensure the storage is synced when any of the callbacks are called. I freely admit the lack of documentation, and if this is a comment you want to add in the driver definition file I would gladly accept it. However, up to this point we've only had a single driver for SMS and it gets this part right. The disk full case we're aware of. Again, this can easily be avoided or minimized by notifying the user of such conditions in advance. This was an important use case for devices with 512K of storage, much less so for devices with 64GB of storage. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Wed, Apr 7, 2010 at 8:05 PM, Denis Kenzior denk...@gmail.com wrote: [...] We prefer to have the id to easily sync with the history plugin, as said in previous mail, if you prefer to not expose the id on IncomingMessage, we may emit an HistoryIncomingMessage signal with the id. Your plugin is free to do anything it wants. The real question is whether you truly need this id or you can simply update the application state based on the IncomingMessage signal only. An use case may explain that better. I have an app that listen for incoming messages and forwards them by mail. After sending X messages the app goes down for some reason, when comes up again there are Y messages in the history plugin becouse Y-X new messages arrived. Well, I want my app to be able to recover from the downtime using the history plugin, and want to avoid to resend the first X messages. A simple way to achieve that is adding the id in the IncomingMessage signal, so the app is able to store it somewhere every time it receives a new message. When the app start, it simply query the history plugin and start processing only messages where id lastsavedid. Of course this is only an easy way to achieve that, I may use the timestamps, but really I do not trust them, it may be I'm missing somethingh here, is there an easy/elegant alternative? A real case may be to powerup the modem at device startup to get already a network registration while the gui comes later, or the user volontary restarts the gui, etc. Regards Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Niko, Infact if a complete dbus api is going to be defined, it would be nice to share it for all history plugins, adding for example an org.ofono.HistoryManager interface to the modem, while if the history plugin has to be used in a different manner, or ofono does not care of that by design, we have to implement the dbus interface in the sqlite plugin directly, forcing our apps to not be generally oFono compliants (at least when not using the sqlite plugin). So today I would consider this to be out-of-scope for the oFono project. We really want to concentrate on being the telephony stack and leave the peripheral aspects to integrators. However, I do realize this is still largely uncharted territory, so any suggestions you have on how oFono core can make your life easier in this space are definitely welcome. In the end some decision has to be made regarding the storage backend. Whether it is MySQL, SQLite, Tracker, EDS, portable black holes, etc. I prefer to leave this to each distribution / platform instead of mandating a solution. In the end we're telephony experts, none of us is an expert with contacts or databases. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
On Wed, Apr 7, 2010 at 9:33 PM, Denis Kenzior denk...@gmail.com wrote: [...] I suggest storing this information directly in the history database. E.g. Read / Unread flag. Ideally the application can access the database concurrently to mark the message as Read / Unread. If not, then some sort of DBus API will be required. At first impact we thinked on this solution, but there are two different use cases, for example from user perspective a message may be read/unread, and that's an information that has a global sense and has to stay in the history db (remember my first mail on this topic when I talked about metadata), but from application perpective a message may be processed or not and IMHO that's an information that should manage the application itself. When the app start, it simply query the history plugin and start processing only messages where id lastsavedid. Why not query the database directly? Yes, but apps cannot determine the last processed id, I know we may query the db every time an IncomingMessage arrives and do some magic (and assumptions, for example that the signal is emitted always after the message reach the history plugin, or doing periodically a cpu wasting query). But thats seems quite weird, so I think finally we'll go providing a dbus interface to the history plugin with a signal giving the needed id when a new message arrives. I understand all that may be out of the ofono scope, but if ineterested we'll keep you updated on our further development and provide patches that may be interesting for other users. Keep up the good work and regards! Niko ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Bastian, Waldo ha scritto: The message handling in this patch seems to be vulnerable to SQL injection attacks. See http://en.wikipedia.org/wiki/SQL_injection Cheers, Waldo Hi Waldo, I didn't think of a message carrying an SQL injection :) Honestly I would use prepared statement since start of the job but I didn't manage how to do them in SQLite but I agree with you they're more secure and the code is cleaner, so I converted the source to them after studying their use. Thank you for your suggestion. Best Regards, Dario. ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/2] Added SQLite history plugin
Bastian, Waldo ha scritto: The message handling in this patch seems to be vulnerable to SQL injection attacks. See http://en.wikipedia.org/wiki/SQL_injection Cheers, Waldo Hi Waldo, I didn't think of a message carrying an SQL injection :) Honestly I would use prepared statement since start of the job but I didn't manage how to do them in SQLite but I agree with you they're more secure and the code is cleaner, so I converted the source to them after studying their use. Thanks, much better :-) Cheers, Waldo ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Added SQLite history plugin
Hi Dario,
+#define SQL_HISTORY_DB_PATH STORAGEDIR /ofono_history.sqlite
+#define SQL_HISTORY_DB_SQL STORAGEDIR /oFono_History_DB.sql
So I have my concerns about storing this in /. This should be in a proper
directory, like /var/lib/ofono or something like that.
+static int sqlite_history_probe(struct ofono_history_context *context)
+{
+ char *execerror;
+
+ ofono_debug(SQLite History Probe for modem: %p, context-modem);
+
+ if (sqlite3_open(SQL_HISTORY_DB_PATH, db) != SQLITE_OK) {
+ ofono_debug(Error opening DB: %s, sqlite3_errmsg(db));
Please use DBG macro instead of ofono_debug.
+ sqlite3_close(db);
+ return -1;
+ }
+
+ if (sqlite3_exec(db, SELECT_CALLS, NULL, NULL, execerror) != SQLITE_OK)
{ + char *sqlscript;
Should be
if (sqlite3_exec == SQLITE_OK)
return 0;
Then the rest of the if statement follows unnested.
+ GError *sqlerror = NULL;
+
+ ofono_debug(Creating DB);
+
+ g_file_get_contents(SQL_HISTORY_DB_SQL, sqlscript, NULL,
sqlerror);
+
+ if (sqlerror != NULL) {
+ ofono_debug(Error opening sql script: %s,
sqlerror-message);
+ g_error_free(sqlerror);
+ return -1;
+ }
+
+ if (sqlite3_exec(db, sqlscript, NULL, NULL, execerror) !=
SQLITE_OK) {
+ ofono_debug(Error executing sql script: %s,
execerror);
+ sqlite3_free(execerror);
+ g_free(sqlscript);
+ return -1;
+ }
+
+ g_free(sqlscript);
+ }
+
+ return 0;
+}
+
+static void sqlite_history_remove(struct ofono_history_context *context)
+{
+ ofono_debug(SQLite History Remove for modem: %p, context-modem);
+
+ if (db != NULL)
+ sqlite3_close(db);
+}
+
So remember that the history plugin is instantiated for each and every modem,
so you will be sqlite_open/sqlite_closing the db in each instance. Not really
sure if this is what you want, or whether you want a reference-counted
database connection here.
Another thing to consider is that you might want to store the IMSI of the
modem along with the history information for every call / sms event. That way
when a SIM card is changed, the user can be shown a different set of call / sms
history.
Finally, you might want to set limits on the number of entries in the
database, and expire them as the limit is reached. Otherwise you'd need to
expire them periodically from e.g. cron, but would need to release control of
the database during that time.
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
