kfirdm / csipsimple

Automatically exported from code.google.com/p/csipsimple
0 stars 1 forks source link

Expert wizard: Data Type not initialized with account DB value #77

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create a new account with the Expert Wizard
2. Go to the Data Type and set to Digest
3. Save the account, note that it registers correctly
4. Re-edit the account
5. Go to Data Type again

What is the expected output? What do you see instead?

Expected to see Digest, saw Plain Password

What version of the product are you using? On what operating system?

build 128

Please provide any additional information below.

I will find this later this morning, I must go now...

ALSO: The Data Type probably should default to Digest as that is what's used in 
the vast majority of cases:

int ctype = ci.getData_type();
if (ctype == pjsip_cred_data_type.PJSIP_CRED_DATA_PLAIN_PASSWD.swigValue()) {
    accountDataType.setValueIndex(0);
} else if (ctype == jsip_cred_data_type.PJSIP_CRED_DATA_DIGEST.swigValue()) {
    accountDataType.setValueIndex(1);
} else if (ctype == pjsip_cred_data_type.PJSIP_CRED_DATA_EXT_AKA.swigValue()) {
    accountDataType.setValueIndex(2);
} else {
    accountDataType.setValueIndex(1);   // rbd default to DIGEST not PLAIN
}
                                  ^
                                  |

Original issue reported on code.google.com by dc3de...@gmail.com on 9 Jul 2010 at 12:40

GoogleCodeExporter commented 9 years ago
Problem comes from the FIXME in the Expert.java file. I'll fix it.
(I use to write some FIXMES sometimes in my files.... :) but not always the 
time to fix it :D )

Though, default value remains plain password. I think it's the most common data 
that is set (it's the default in pjsip apps too).
Be careful : data_type is different from authentication scheme (there is maybe 
something not clear here).

Besides if registration is ok, it means that you should also use plain password 
since actually it register always with plain password.
Another point : for now EXT_AKA is not really supported since we need to fill 
other fields for which there is no interface yet.

Original comment by r3gis...@gmail.com on 9 Jul 2010 at 1:17

GoogleCodeExporter commented 9 years ago
OK on the default for plain password, though all of the SIP traffic I have ever 
seen uses Digest (data type)...

But these two fields (scheme and data type) are also being overridden by the 
SIP stack's defaults when the wizard is created. First they are loaded from the 
saved preferences:

    accountDataType = (ListPreference) findPreference("data_type");
    accountScheme = (ListPreference) findPreference("scheme");

Then later, those values are always REPLACED by the SIP stack's defaults:

{
    String scheme = ci.getScheme().getPtr();
    if (scheme != null && !scheme.equals("")) {
    accountScheme.setValue(ci.getScheme().getPtr());
    } else {
    accountScheme.setValue("Digest");
    }
}
{
    int ctype = ci.getData_type();
    if (ctype == pjsip_cred_data_type.PJSIP_CRED_DATA_PLAIN_PASSWD.swigValue()) {
        accountDataType.setValueIndex(0);
    } else if (ctype == pjsip_cred_data_type.PJSIP_CRED_DATA_DIGEST.swigValue()) {
        accountDataType.setValueIndex(1);
    } else if (ctype == pjsip_cred_data_type.PJSIP_CRED_DATA_EXT_AKA.swigValue()) {
        accountDataType.setValueIndex(2);
    } else {
        accountDataType.setValueIndex(0);
    }
}

The result of this is, whenever you edit an account that uses the Expert 
wizard, you MUST go in and check the scheme and data type, and re-set it if the 
stack default is not what you want. Otherwise, you will save the account with 
stack defaults instead of your desired selections. This is similar to the 
problem in Issue 75, and in the same area of Expert.java.

Original comment by dc3de...@gmail.com on 9 Jul 2010 at 1:48

GoogleCodeExporter commented 9 years ago
See my comment on issue 75. I think you misunderstood what is the flow on the 
expert wizard

Original comment by r3gis...@gmail.com on 9 Jul 2010 at 1:59

GoogleCodeExporter commented 9 years ago
Strange... when creating a new account, with an existing account, the 
accountRegTimeout is initialized the the setting from the other account???

Original comment by dc3de...@gmail.com on 9 Jul 2010 at 2:02

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
OK, I read your comment, and I think I understand now. I will update to your 
latest source and test here.

Original comment by dc3de...@gmail.com on 9 Jul 2010 at 2:04

GoogleCodeExporter commented 9 years ago
Btw, you're probably right when creating a new account, maybe values from 
existing account are populated.
That's a side effect of the use of the preference system to populate views. 
There is probably a todo somewhere on my code related to this issue. I have to 
find a way to clean values when quit the preference screen and to restore 
current values when orientation of the screen is changed.

Googlecode svn is dead right now so I can't update my last changes but i'll do 
it as soon as possible.

Original comment by r3gis...@gmail.com on 9 Jul 2010 at 2:26

GoogleCodeExporter commented 9 years ago
OK, with Expert.java rev. 132:

1. Start CSipSimple with no accounts
2. Create an account, set the Data Type to Digest
3. Save the account and close the wizard, see the account in the list
4. Edit the account
5. Open the Data Type preference

Expected: Digest (previously set)
Saw: Plain Password

I don't know whether Digest was saved when the account was created. It may be 
that the popup menu is not being initialized with the saved preference? 

Original comment by dc3de...@gmail.com on 9 Jul 2010 at 2:31

GoogleCodeExporter commented 9 years ago
No was not saved in fact. That's part of the modification I didn't succeed to 
push on google code since google svn respond error 500.

I'll notify you on this thread when my modification will be on the svn.

I've also deactivated AKA type since even if it can be choose there is missing 
fields to use it.

Original comment by r3gis...@gmail.com on 9 Jul 2010 at 2:35

GoogleCodeExporter commented 9 years ago
This is really wonderful! Once your work is stabilized, I have some merging to 
do here. 

Original comment by dc3de...@gmail.com on 9 Jul 2010 at 2:49

GoogleCodeExporter commented 9 years ago
Changes look good here on r142.

Original comment by dc3de...@gmail.com on 10 Jul 2010 at 4:19

GoogleCodeExporter commented 9 years ago
Well, fixed :)
It reduces the number of open issues ... cool :D

Original comment by r3gis...@gmail.com on 10 Jul 2010 at 4:30