Closed enoch85 closed 9 years ago
db/smsmapper.php#45 reads:
$mbox = SmsMapper::$mailboxNames[$row["sms_mailbox"]];
SmsMapper::$mailboxNames contains the following entries:
private static $mailboxNames = array(0 => "inbox", 1 => "sent", 2 => "drafts");
so 0-2 are valid indices. the data $row["sms_mailbox"] is comming from the db table PREFIXocsms_smsdatas
so any value is inserted either by the sync client (via writeToDB()) or by the user directly to the db.
looking at the sync client, there are the follwing four codes: MailboxID.java
public enum MailboxID {
INBOX,
SENT,
DRAFTS,
ALL,
}
so, values 0-3.
could 3 be the issue? SmsFetcher.java shows, that in the first place (fetchAllMessages) only values 0-2 are used, and in the second place (bufferizeMailboxMessages) only values 0-2 are accepted.
public SmsFetcher(Context ct) {
_lastMsgDate = (long) 0;
_context = ct;
_existingInboxMessages = null;
_existingSentMessages = null;
_existingDraftsMessages = null;
}
public JSONArray fetchAllMessages() {
_jsonDataDump = new JSONArray();
bufferizeMailboxMessages(MailboxID.INBOX);
bufferizeMailboxMessages(MailboxID.SENT);
bufferizeMailboxMessages(MailboxID.DRAFTS);
return _jsonDataDump;
}
private void bufferizeMailboxMessages(MailboxID mbID) {
String mbURI = mapMailboxIDToURI(mbID);
if (_context == null || mbURI == null) {
return;
}
if (mbID != MailboxID.INBOX && mbID != MailboxID.SENT &&
mbID != MailboxID.DRAFTS) {
Log.e(TAG,"Unhandled MailboxID " + mbID.ordinal());
return;
}
not sure, if in java arguments (enum) are passed as reference or by value. in case of a reference there could only be that android.net.Uri is changing the mbox value. but i don't think so.
so in the current code i can't find a reason why there should be an wrong index in mbox.
has the error been confirmed? is it still happening? what are the client/server versions of the app and the oc-server?
Ubuntu Server 12.04
Database: Apache/2.2.22 (Ubuntu) MySQL: 5.5.41 PHP version: PHP 5.5.9-1ubuntu4.5 (cli) (built: Oct 29 2014 11:59:10)
ownCloud version: (see ownCloud admin page) 8.0 Updated from an older ownCloud or fresh install: Updated from 5, 6, 7
List of activated apps: Activity Calender Contacts Deleted files Documents First Run Wizard Full Text Search PDF Viewer Pictures Share Files Text Editor Updater Versions Video Viewer ownCloud SMS
Are you using external storage, if yes which one: local/smb/sftp/... No Are you using encryption: yes/no No
Browser: Version 39.0.2171.71 m Operating system: Windows 8.1 Sync Client 1.7.1 stable
And yes, still happening.
would you mind to dump:
SELECT sms_mailbox
FROM `oc_ocsms_smsdatas`
GROUP BY sms_mailbox
@martin-rueegg
great! now, i would be interested in the records with sms_mailbox = 3
. please check if you can share some data of it. 3 is the enum for MailboxID.ALL
.
could it be possible that the data has been synced to the server with an older client app?
as a workaround you could update the given records to have sms_mailbox = 2. the error should then be disappearing while you still can identify the records (having a mbox value of 2 instead of 3).
UPDATE `oc_ocsms_smsdatas` SET `sms_mailbox` =2 WHERE `sms_mailbox` =3
@martin-rueegg MySQL shell isn't my expertise. ;) Please send the commands and I will execute them.
if there is no sensitive data (please check or edit data):
SELECT * FROM `oc_ocsms_smsdatas` WHERE `sms_mailbox` = 3
otherwise avoid the message body using:
SELECT `id`, `user_id`, `added`, `lastmodified`, `sms_id`, `sms_address`, `sms_date`, `sms_flags`, `sms_type` FROM `oc_ocsms_smsdatas` WHERE `sms_mailbox` = 3
UPDATE `oc_ocsms_smsdatas` SET `sms_mailbox` =2 WHERE `sms_mailbox` =3
then check logs while syncing.
Checked, and sms_mailbox 3 is empty!
Will do a resync and put the output here.
well, according to your first output, there is at least one record with sms_mailbox = 3. please still past the output of:
SELECT sms_mailbox, count(*)
FROM `oc_ocsms_smsdatas`
GROUP BY sms_mailbox
and
SELECT `id`, `user_id`, `added`, `lastmodified`, `sms_id`, `sms_address`, `sms_date`, `sms_flags`, `sms_type`
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` not in (0, 1)
I did the workaround as you said.
SQL-resultat
Värd: localhost
Databas: owncloud
Skapad: 13 feb 2015 kl 01:28
Genererad av: phpMyAdmin 3.4.10.1deb1 / MySQL 5.5.41-0ubuntu0.12.04.1
SQL-fråga: SELECT sms_mailbox, count(*) FROM `oc_ocsms_smsdatas` GROUP BY sms_mailbox LIMIT 0, 30 ;
Rader: 3
sms_mailbox count(*)
0 1218
1 1421
2 2
SQL-resultat
Värd: localhost
Databas: owncloud
Skapad: 13 feb 2015 kl 01:29
Genererad av: phpMyAdmin 3.4.10.1deb1 / MySQL 5.5.41-0ubuntu0.12.04.1
SQL-fråga: SELECT `id`, `user_id`, `added`, `lastmodified`, `sms_id`, `sms_address`, `sms_date`, `sms_flags`, `sms_type` FROM `oc_ocsms_smsdatas` WHERE `sms_mailbox` not in (0, 1) LIMIT 0, 30 ;
Rader: 2
id user_id added lastmodified sms_id sms_address sms_date sms_flags sms_type
273978 enoch 0000-00-00 00:00:00 0000-00-00 00:00:00 2530 +46704330831 1422056574639 01 1
1067830 enoch 0000-00-00 00:00:00 0000-00-00 00:00:00 2960 +46737451044 1423081307418 01 1
ok.
do you have a rooted phone? can you open your sms-database on the phone and see what the records for messages with id 2530 and 2960 are? are they MMS?
No more error. :D
I have a Nexus 5, not rooted. Sorry.
New error though when opening the Web UI SMS:
Error PHP Undefined offset: 0 at /var/www/owncloud/apps/contacts/lib/backend/shared.php#90 2015-02-13T00:46:59+00:00
Error PHP Undefined offset: 0 at /var/www/owncloud/apps/contacts/lib/backend/shared.php#90 2015-02-13T00:46:59+00:00
Error PHP Undefined offset: 0 at /var/www/owncloud/apps/contacts/lib/backend/shared.php#90 2015-02-13T00:46:59+00:00
Error PHP Undefined offset: 0 at /var/www/owncloud/apps/contacts/lib/backend/shared.php#90 2015-02-13T00:46:59+00:00
Error PHP Undefined offset: 0 at /var/www/owncloud/apps/contacts/lib/backend/shared.php#90 2015-02-13T00:46:59+00:00
well, this might be a "new" error, or an error that is unrelated to the first one. but since the first error is now "solved", the program continues and runs in a second one.
obviously you have multiple contact address books. if i understand the code of lib/backend/shared.php correctly, you might even have a shared addressbook.
not sure, if this is an ocsms issue. see (https://github.com/nerzhul/ownCloud-SMS-App/issues/23) on the comment regarding multiple addressbooks.
No, one adresssbook, but other adressbooks for other users on the server, so maybe that's the issue?
Thank you for helping out anyway! Any permanent fix for this in next release?
well, it is not sure, where the error came from. so the issue is not "fixed"! :worried:
the SQL we run was only a workaround to see if my assumption is correct. according to the current code i cannot understand, where this mbox=3 came from! that's why i asked if you can recognize the messages you found by the sql-command:
SELECT *
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` not in (0, 1)
it might help to see the data on your phone. but i have no idea how to access it, if your phone is not rooted. one option would be to delete all the messages on your server and sync them again. i could send you the SQL for purging the messages on your server. all the SMS would then be re-uploaded to the server again. you would have to stop the sync service on the phone, and then reactivate it.
it would be interesting in the sense that we could figure out if it was an one-time error (when synching the first time) ore if the data in your phone are special (if the error will pop up again after resync).
ready?
i guess if you don't have any shared contacts form other users, it should not be a problem. but i did not dig into that so far. (today is my first day in OC and ocsms coding!)
What table do I drop to redownload?
Btw, it's regular SMS not MMS.
read the instructions. in case of questions pls don't hesitate to ask.
1 in phpmyadmin select the table oc_ocsms_user_datas
2 on the top menu select Operations
3 on the right hand side use the copy table function to copy the table with structure and data (default) to oc_ocsms_user_datas_sav
(we will use that table later to restore the message-read-information).
4 check that the new table oc_ocsms_user_datas_sav
exists and has records
5 repeat steps 1 to 4 for the oc_ocsms_smsdatas
-> oc_ocsms_smsdatas_sav
(just for the sake of safety)
6 turn off sync on mobile
7 run the following SQL commands (this will empty the two tables completely):
TRUNCATE oc_ocsms_smsdatas;
TRUNCATE oc_ocsms_user_datas;
8 turn on sync on mobile 9 wait for sync to complete 10 check if messages appear in web UI (will be all unread. don't worry) 11 restart sync on mobile in order to do a second sync (the error won't appear during first sync). and check the log files. 12 run the following SQL commands and paste the output:
SELECT sms_mailbox, count(*)
FROM `oc_ocsms_smsdatas`
GROUP BY sms_mailbox;
SELECT `id`, `user_id`, `added`, `lastmodified`, `sms_id`, `sms_address`, `sms_date`, `sms_flags`, `sms_type`
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` not in (0, 1)
13 run the following command to restore the read-message-info:
INSERT INTO `oc_ocsms_user_datas`(`user_id`, `datakey`, `datavalue`)
SELECT * FROM `oc_ocsms_user_datas_sav` WHERE 1
14 check in web-ui if messages are marked as read again.
i did everything here on my side. so it should be safe.
twelve.
sms_mailbox count(*)
0 1119
1 1301
SELECT `id` , `user_id` , `added` , `lastmodified` , `sms_id` , `sms_address` , `sms_date` , `sms_flags` , `sms_type`
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` NOT
IN ( 0, 1 )
LIMIT 0 , 30
Jumped a few steps, didn´t create .sav
Now it's time for bed. Good night :)
well, there seems to be either some messages missing or the sync is not complete. before the re-synch you reported:
sms_mailbox count(*) 0 1218 1 1421 3 2
now you only had:
sms_mailbox count(*) 0 1119 1 1301
no missing approx 200 msg.
how about:
SELECT `id` , `user_id` , `sms_id` , `sms_address` , `sms_date` , `sms_flags` , `sms_type`
FROM `oc_ocsms_smsdatas`
WHERE `sms_id` IN ( 2530, 2960 )
can you (tomorrow) post the output?
Now it's time for bed. Good night :)
jep. you're right! same, same.
Hi. Thanks for your investigations. MMS aren't read and synced with owncloud
I deleted some messages on the phone, that's why. :)
Med vänlig hälsning Daniel Hansson
www.en0ch.se
Skickat från Google Nexus 5 Den 13 feb 2015 03:35 skrev "Martin Rüegg" notifications@github.com:
well, there seems to be either some messages missing or the sync is not complete. before the re-synch you reported:
sms_mailbox count(*) 0 1218 1 1421 3 2
now you only had:
sms_mailbox count(*) 0 1119 1 1301
no missing approx 200 msg.
how about:
SELECT
id
,user_id
,sms_id
,sms_address
,sms_date
,sms_flags
,sms_type
FROMoc_ocsms_smsdatas
WHEREsms_id
IN ( 2530, 2960 )can you (tomorrow) post the output?
Now it's time for bed. Good night :)
jep. you're right! same, same.
— Reply to this email directly or view it on GitHub https://github.com/nerzhul/ocsms/issues/39#issuecomment-74195372.
yeah, i guess so. hope you don't mind not having them on the server any more.
still, could you check if the two messages that caused the error are still in your message set? if they're gone now, we will have to close this issue, as there is no way to find out, what was the cause of it. in fact, even if the messages are there we wouldn't be able to do so.
i see two possible reasons for the issue:
either way we can not reproduce the error any more. i have reviewed the code (except for the android libraries) and could not figure out, where the number 3
could have originated from. thus i would suggest to close the issue for now.
Thanks for your report and your time. I think this 3 comes from the old version which doesn't filter types
Thanks for helping out!
welcome. :-)
well then, if an old version (or a user, in an other case) could have entered wrong values, would it then make sense to change getAllIds() in one of the following ways?
'WHERE user_id = ? AND sms_mailbox IN (' . implode(', ', array_keys(SmsMapper::$mailboxNames)) . ')'
(yes, we would use a parameter ?
. this here is just a draft)array_key_exists($row["sms_mailbox"], SmsMapper::$mailboxNames)
(Line 45) and then act accordingly (e.g. updating the mbox in database to a valid default (suggestion: 2 for 'drafts'))the first variant would cause the client to resend the given message. thus writeToDB() would have to be altered in order to update existing entries.
I will modify the sync process to improve the API in next app version , we can think about a modification for sync process V2
{"reqId":"cfc92de7499534564c57fab66d28f999","remoteAddr":"109.58.144.140","app":"PHP","message":"Undefined offset: 3 at \/var\/www\/owncloud\/apps\/ocsms\/db\/smsmapper.php#45","level":3,"time":"2015-02-14T06:40:52+00:00"}
Still same error.
@enoch85 , what is the version of your sms-sync client? the current v0.17.0?
can you tell us something about the nature of the sms found by
SELECT *
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` NOT
IN ( 0, 1 )
guess we still should think about one of the solutions suggested in https://github.com/nerzhul/ocsms/issues/39#issuecomment-74243202
@martin-rueegg The versions are 1.17.0 and 1.4.2.
The SQL didn't give any results.
@enoch85, where did you get this version string 1.17.0
from? are we sure we're both talking about the owncloud SMS android client?
as to my https://github.com/nerzhul/ocsms/issues/39#issuecomment-74159945 the index is coming from the db table *PREFIX*ocsms_smsdatas
(default oc_ocsms_smsdatas
). the sms_mailbox
field is a smallint and does not allow NULL values. so how could the error be thrown, if the query
SELECT *
FROM `oc_ocsms_smsdatas`
WHERE `sms_mailbox` NOT
IN ( 0, 1 )
doesn't return any results? :hushed:
@enoch85, do you have any dbtableprefix
in your config.php
, if so, what's it's value?
regarding client version: according to your screenshot it reads v0.17.0. that's fine. tnx.
@enoch85 please run again
SELECT sms_mailbox, count(*)
FROM `oc_ocsms_smsdatas`
GROUP BY sms_mailbox;
sms_mailbox count(*) 0 1125 1 1309
Hmm, look at the time. Maybe this is before we fixed it?
what time? we havn't fixed anything yet. we have just re-synced all messages in order to see, if the android client is sending some "special" data. according to the code, the data causing the error comes from the database. the data in the database comes from the client. so my idea was to find out, where the erroneous data is coming from. now, according to your information, there is no erroneous data in your database. so the error "is not possible". ah, by "time" you mean the time of the error message? it states "2015-02-14T06:40:52+00:00". the "fix" we did on 2015-02-13. it's very weird.
sorry for asking: are you sure you run the SQL on the same database, the oc instance creating the errorlog is accessing as well?
@martin-rueegg haha, yes I'm sure.
@nerzhul just noticed, that the following functions in the client do not check the mailbox. so there could be a mbox=3 message flawing to the server, right? see:
wouldn't it make sense to merge these two functions with the third of the same type?
as they mainly differ in the way they select the messages
if the second parameter Long sinceDate
would be optional (not knowing how to do this in java), then we could use one single function under the thee following conditions:
sinceDate = 0
sinceDate
the same way it does it nowsinceDate
alone, defaulting it to NULL or EMPTY or alike.this would ease the maintenance of the code, IMHO.
@enoch85, i know it was a funny question. but sometimes even Pros overlook a tiny little thing. at least i do. :smile:
i really don't know what to say. according to the code and your data i simply cannot understand why the error is occuring. can it be something in \OCP\DB::prepare, so that the result array is "strange"?
i'm stuck.
In fact we trust the database content, because the only write is done by sync which permits only type 1 and 2. Could you provide a PR with the optimization you'll find we could discuss there.
re: optimization: ok, i can do so, but please tell me how to define a optional parameter with a default value in java. (not sure if i'll get there today though ...)
re: database write
if either bufferizeMessagesSinceDate() (//Used by ConnectivityChanged Event) or getLastMessage() (// Used by Content Observer) are called with MailboxID mbID = 3 then mapMailboxIDToURI() returns content://sms
and mbID.ordinal()
returns 3 (which is stored to mbox in the JSON) which in turn might be wrong as to my understanding content://sms
would return any mailbox (inbox, sent, draft, ...). instead of mbID.ordinal(), the actual value in the record shoud be used.
and at the server we safe any value coming from the client to the db. so a 3 could result as shown above.
(found about optional parameters)
you are right. I found where the issue can happen. It's in SmsObserver.onChange, we check the last message for all mailboxes when the connectivity changed. And then if the mailbox is other than the required mailboxes, it worked. I need to fix the issue now, and i publish a patch on Google Play Store soon.
i'll try to propose the merge of the functions as discussed previously. so we will only have one main function to maintain. i'll do it today. could you wait with sending the patch to google play store? btw: i think we should still do a check on the server side as well, as we should never trust the data coming from the internet. someone might build a compatibe client and use the server-api.
No release for google play store at this time. Server must be tagged before client, to support the new bugfixes/features.
I reeally wonder what this is:
Running 1.4.2.