Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crashes in XMPPvCardTempEmail and XMPPvCardTempTel initialize #1162

Merged

Conversation

beribas
Copy link
Contributor

@beribas beribas commented Apr 7, 2020

Fixes #1050 and #1118.

Setter methods in XMPPcVardTempEmail and XMPPcVardTempTel had typing mistakes. Must have been happened sometime during 3.7 to 4.0 transition.
These types do not allow adding properties, that's why the allocated size is compared to the size of NSXMLElement in initialize.

+ (void)initialize {
        // .....
	size_t superSize = class_getInstanceSize([NSXMLElement class]);
	size_t ourSize   = class_getInstanceSize([XMPPvCardTempEmail class]);
	
	if (superSize != ourSize)
	{
		XMPPLogError(@"Adding instance variables to XMPPvCardTempEmail is not currently supported!");
		
		[DDLog flushLog];
		exit(15);
	}
}

Every added property has to have matching get and set overwrites, otherwise ObjC will allocate memory for the property and the size check will fail.
The linked issue describe that behavior.

I also added missing implementation for TEL and few tests for XMPPvCardTemp.

Copy link
Collaborator

@chrisballinger chrisballinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks great.

Do you think you could use XMLElement instead of DDXMLElement?


import XCTest
import XMPPFramework

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import KissXML

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like I don't have to import anything at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, it's probably implicitly re-exported by XMPPFramework.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, hm that's weird that you don't even need to import XMPPFramework.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I also don't really understand why. :)

Xcode/Testing-Swift/XMPPvCardTempTests.swift Outdated Show resolved Hide resolved
@beribas
Copy link
Contributor Author

beribas commented Apr 7, 2020

O yes, sure, didn't know about the XMLElement alias.

@chrisballinger
Copy link
Collaborator

Awesome, thank you!

@chrisballinger chrisballinger merged commit 9d625f5 into robbiehanson:master Apr 8, 2020
@beribas beribas deleted the fix_crashing_vcardtemp_types branch April 8, 2020 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XMPPvCardTempEmail exits process on +initialize()
2 participants