robbiehanson / XMPPFramework

An XMPP Framework in Objective-C for Mac and iOS
Other
5.91k stars 2.09k forks source link

Substitute ICU's stringprep for libidn #217

Open andrewtj opened 11 years ago

andrewtj commented 11 years ago

I've slapped together a replacement for the LibIDN class built on ICU's stringprep functions. The code is in a gist for the moment as I'm not keen on updating all the samples, docs, etc to use it before knowing whether there's any interest in seeing it merged.

To build it, grab the files from the gist, icu4c-51_2-src.tgz and icudt51l.zip by selecting rfc3491.spp, rfc3920node.spp and rfc3920res.spp from Miscellaneous Data. Then issue make -j 4 or whatever is appropriate for your machine.

To use it, add ICU4XMPPFramework.h, ICU4XMPPFramework.a and libstdc++ to a project and then swap out LibIDN for ICU4XMPPFramework in XMPPJID.m.

It's not been tested much. I've run it with an OS X app and an iOS app in the simulator and that's about it.

Any interest?

andrewtj commented 11 years ago

Not a nibble?

ObjColumnist commented 11 years ago

I suppose you will have to sell us the benefits :smile:

avaidyam commented 11 years ago

I'd rather just rewrite it and get rid of the dependencies... but that's just me ;)

andrewtj commented 11 years ago

@ObjColumnist the appeal for me is the licensing terms. If you're happy with libidn's conditions there's no other major benefits. (Well there's this but I haven't verified it myself.)

@galaxas0 No one's stopping you.

avaidyam commented 11 years ago

Except time itself. ;)

andrewtj commented 11 years ago

@galaxas0 That's precisely why I enlisted ICU :wink:

avaidyam commented 11 years ago

Hah! I see :) I think going with ICU over LibIDN seems like a good idea too.

andrewtj commented 11 years ago

To me the ICU License seems like a clear-win over the LGPLv3 (current libidn license) for development on Apple platforms.

andrewtj commented 11 years ago

I'm no lawyer but as I see it using LGPL licensed code in an App-store (or signed or statically linked) app either conflicts or brings extra release requirements due to section 4 of the current license and 6 in the old license.

As I don't want to do this sort of thing with the apps I work on (and I'm not sure it's sufficient in any case) I'm going with ICU in place of libidn. Unless there's further correspondence to indicate otherwise in the next day or so, I'll assume there's no interest and close this issue.

ObjColumnist commented 11 years ago

@andrewtj I would like to keep this issue open as it is something that needs addressing, but I would also like to explore the possibility of using just Objective-C.

If your going with ICU in your project, that will be great as you will find out if it has any adverse effects.

avaidyam commented 11 years ago

I can rewrite a majority of the requirement in ObjC. I'll try that out.

paulmelnikow commented 11 years ago

Having a compatibly licensed product to use as a reference implementation should make it easier to reimplement in ObjC. I had flipped through the Unicode docs once before and they're pretty dense.

avaidyam commented 11 years ago

Aye, then I'll use this ICU rewrite and "fill in the methods" using pure Objective-C, referencing stringprep along the way.

paulmelnikow commented 11 years ago

Cool. When you get some code together I'm happy to take a look and contribute, particularly in terms of writing test code and refactoring for readability and such.

Does ICU have any test code? Else, come to think of it, does libidn? LGPL-licensed tests would be acceptable.

avaidyam commented 11 years ago

Great! I hope this PR will be open until then, I'll try and finish this up today.

I guess I'll use the whole XMPPFramework as test code, haha!

paulmelnikow commented 11 years ago

@galaxas0 I think it's worth building this in a new repository. This is general purpose code, useful to other ObjC projects XMPP-related and otherwise, and it saves others the effort of extracting it later. We can import it into this repository (or better, link it as a submodule) once it's done.

Let me know if you'd like a hand setting it up this afternoon. I won't be online much this weekend but should have a little time Monday night or Tuesday.

paulmelnikow commented 11 years ago

@andrewtj if one of us sets up a new repo, would you mind committing your work from the gist?

avaidyam commented 11 years ago

I'll start up a new repo today then. Just for the string prep class.

andrewtj commented 11 years ago

@paulmelnikow Sure, just fork/clone the gist repo (https://gist.github.com/5843559.git). Consider it licensed under the same terms as XMPPFramework (BSD).

paulmelnikow commented 11 years ago

@galaxas0 Did you get to do that?

avaidyam commented 11 years ago

I started work on it, but I'm hopelessly lost trying to "integrate" LibIDN's stringprep functions.

paulmelnikow commented 11 years ago

Maybe it's more effort than it's worth to reimplement it. I'd like to try to integrate ICU on my end and see what that looks like.

To that end I suggest we rename the class from LibIDN to XMPPStringPrep – see my pull request #225 – and then work on plugging in an alternative.

avaidyam commented 11 years ago

Yep, I called it XMPPStringPrep as well.

andrewtj commented 11 years ago

@galaxas0 forgive the pedantry, but working from LibIDN somewhat implies you're making a derived work… :wink:

I'm not sure what the priorities of the project are, but for my $0.02, removing an LGPL'd dependancy is up there and while writing new code to avoid a new dependancy is laudable, there's other areas that could benefit more from some time and attention.

paulmelnikow commented 11 years ago

@andrewtj Did you know that iOS and Mac OS ship with a version of ICU? RegexKitLite seems simply to use -licucore to link against it. I've seen some reports that linking to icucore has caused apps to be rejected, but according to the author of RegexKitLite this hasn't been a problem for his users. The library doesn't install with headers, and I haven't determined what version is available, but it might be worth trying.

avaidyam commented 11 years ago

That's completely agreeable, but I know next to nothing about stringprep. Once I have some sort of an implementation, I can rewrite it for efficiency :)

avaidyam commented 11 years ago

Hey wait! http://blog.lukhnos.org/post/6441462604/using-os-xs-built-in-icu-library-in-your-own-project

paulmelnikow commented 11 years ago

@galaxas0 If you're going to adapt existing code you should start with ICU instead of LibIDN.

avaidyam commented 11 years ago

@paulmelnikow @andrewtj @ObjColumnist I simply suggest we use the built in ICU framework. /usr/lib/libicucore.dylib.

andrewtj commented 11 years ago

@paulmelnikow Yes - I'd read scattered references to people getting rejected for using it though. Given ICU's binary compatibility, I'd guess if you stick to the C API it might maybe be okay. I'd rather err on the side of caution though which is why I went down the route of building standalone with a suffix.

avaidyam commented 11 years ago

@andrewtj Can you modify the gist to support the built-in library? Or should I try this at home first? ;)

andrewtj commented 11 years ago

@galaxas0 I'm not inclined to do that. If you strip off the data libraries and grab the profiles from disk you should be right to go.

avaidyam commented 11 years ago

@andrewtj Alright, I'll give it a go myself then!

andrewtj commented 11 years ago

I've converted the gist into a full-fledged repo and renamed the class XMPPStringPrep as per #225.

chrisballinger commented 10 years ago

Just found this the other day, would this be useful? https://github.com/wordpress-mobile/NSURL-IDN