VoIPGRID / VialerSIPLib

An Objective-c wrapper for PJSIP
GNU General Public License v3.0
133 stars 69 forks source link

Crash when optional properties are not implemented for SIPEnabledUser protocol #59

Closed miguelrc86 closed 7 years ago

miguelrc86 commented 7 years ago

Version

2.5.0

File / Feature

VialerSIPLib.h

Expected behaviour

The app should not crash when avoiding optional properties conforming to SIPEnabledUser protocol

Actual behaviour

The app crashes when avoiding optional properties conforming to SIPEnabledUser protocol

Stacktrace / Error message

2017-01-28 20:53:43.107380 Nommox[2348:740395] -[Nommox.SipUser sipProxy]: unrecognized selector sent to instance 0x1740aa4a0

Other info

I noticed the declaration for sipProxy property looks like this:

@property (readonly, nonatomic) NSString * _Nonnull sipProxy;

To my understanding, it should use the _Nullable type annotation so I can pass nil in my custom class. Another solution would be to validate the length of the accountConfiguration.sipProxyServer on the VSLAccount but maybe that's not so pretty :-)

Also I think it would be good if you implement a validation for the existence of that property when doing the account configuration to avoid this unrecognized selector crash.

If useful, I'm on a project using both Objective-C and Swift languages.

Thanks in advance, and please let me know if there's something I'm losing here.

miguelrc86 commented 7 years ago

Thanks Bob!

mital87 commented 7 years ago

@bobvoorneveld & @spindleredmer: Still this issue persists in the current version (2.6.0). suppose i don't want to set SipProxy (ie. SipAccountId, SipPassword, SipDomain are set properly) then my app crashed. Please check it once.

bobvoorneveld commented 7 years ago

do you have a traceback/log?

mital87 commented 7 years ago

@bobvoorneveld: Please check below traceback/log.

DEBUG [VSLEndpoint -[VSLEndpoint startEndpointWithEndpointConfiguration:error:]:124] Creating new PJSIP Endpoint instance. 12:59:55.054 os_core_unix.c !pjlib 2.6 for POSIX initialized 12:59:55.055 sip_endpoint.c .Creating endpoint instance... 12:59:55.057 pjlib .select() I/O Queue created (0x7fd3fa83f228) 12:59:55.057 sip_endpoint.c .Module "mod-msg-print" registered 12:59:55.057 sip_transport. .Transport manager created. 12:59:55.057 pjsua_core.c .PJSUA state changed: NULL --> CREATED INFO [VSLEndpoint void logCallBack(int, const char , int):450] pjsua_core.c .pjsua version 2.6 for Darwin-16.4/x86_64 initialized INFO [VSLEndpoint -[VSLEndpoint startEndpointWithEndpointConfiguration:error:]:231] PJSIP Endpoint started succesfully ERROR [VSLEndpoint void logCallBack(int, const char , int):444] pjsua_core.c .Invalid route URI: sip: ERROR [VialerSIPLib -[VialerSIPLib createAccountWithSipUser:error:]:117] Account configuration error: Error Domain=VialerSIPLib.VSLAccount Code=0 "Could not configure VSLAccount" UserInfo={NSLocalizedDescription=Could not configure VSLAccount, NSLocalizedFailureReason=PJSIP status code: 171039} ERROR [DDLogWrapper +[DDLogWrapper logError:]:57] Could not create account. Error:Error Domain=VialerSIPLib.VSLAccount Code=0 "Could not configure VSLAccount" UserInfo={NSLocalizedDescription=Could not configure VSLAccount, NSLocalizedFailureReason=PJSIP status code: 171039} Exiting assertion failed: file /Users/mitals/Desktop/VialerSIPLib-develop/Example/VialerSIPLib/AppDelegate.swift, line 74

bobvoorneveld commented 7 years ago

what is your sipDomain? Might be that that is the problem and it has nothing to do with the proxy (if it is nil, it isn't added, see line: https://github.com/VoIPGRID/VialerSIPLib/blob/develop/Pod/Classes/VSLAccount.m#L126

Try adding a breakpoint to check if the proxy is added or not and what the reg_uri is.

mital87 commented 7 years ago

@bobvoorneveld: I am passing static let Proxy = "" in Keys.swift file. I am getting if (accountConfiguration.sipProxyServer) always true. How can i print reg_uri?

bobvoorneveld commented 7 years ago

Please don't set the proxy to an empty string, we're checking on nil value in the code.

mital87 commented 7 years ago

@bobvoorneveld: Thanks for the support & close the issue.