Re: [PEPr] +1 for Networking::Net_WebFinger

On Fri, Feb 17, 2012 at 3:35 PM, Christian Weiske <cweiske@cweiske.de>wrote: > Hello Till, > > > > 1) What I'm not so sure about is the request code in the > > Net_WebFinger class. Some of the methods seem long and a little > > redundant in terms of code. > > > > Maybe spin it off into a dedicated request class. I have no idea yet, > > if you catch my drift, maybe you can improve this, otherwise I'll > > take a look later and will send you a pull request. > > I know what you mean and already thought about this, but consider one: > the class Net_WebFinger has only one task, it's to deliver the reaction > object. Since there is nothing more it has to do, having the > network request code in there is ok in my eyes. > Yeah, that makes sense. It's why I couldn't come up with anything either. > > > 2) Net_WebFinger::finger() should IMHO throw an exception if no '@' is > > found. > In the identifier? Hm. Didn't think of that yet :) > > > > 3) I'd see if you can use Net_URL2 (which comes with HTTP_Request2) > > for the "isHttps()" check. Speaking of which, is SSL usually a given > > with these things or the exception? If most of the resources are > > non-SSL, I'd run the non-SSL request first. Otherwise it'll be pretty > > expensive. > > > > And/Or offer a way to force "non-SSL" lookup (with finger()). > > Webfinger maps an email address to an openid, which is something that's > a target of hijacking (if I were an attacker). If I can move any > address to my own openid provider, I've won. So using a secure channel > first is the best thing to do. Non-SSL is only a fallback. The > WebFinger specs recommend that behavior. > Alright, I guess whatever works. Forcing "non-ssl" would be nice IMHO. > > > 4) I'm not so sure if I like Net_WebFinger_Error. Are all errors > > usually to be disregarded? Is that why you don't throw Exceptions? Or > > is there another reason I am missing. > Not many hosts have webfingers yet, and not all webfinger/lrdd files > have the same features/links. > > My idea of the package was to make discovery extremely easy, and it > should not get in your way. If you want an openid, just do > Makes sense. > > $wf->finger('user@host')->openid > and nothing more. You don't care if there are errors, because the > openid is null if it can't be found. If you really care about the > errors, which most people don't, because they just want the openid, you > can still access it via the error property. > > That's the reason for no exception throwing: Don't get in the way, but > provide enough information for a detailled analysis. > Makes sense. > > > 5) I'd like to see an autoloader. I'm not sure why you think PEAR1 > > cannot have this. Especially with a 5.3 minimum, require_once seems > > really outdated and would force me to fork for my own uses to use > > another class-loading strategy. > > > > Imho, an autoloader could take care of all that. Enabled by default, > > and if people want to run their own, they could disable it. Then > > there's no need to strip require_once. > > Should we use the PEAR2 autoloading rules then? > I think so. > > > > 6) Careful on the dependency on Cache. ;) (I think we need to work on > > something there and bring the code up to date.) > It's optional, so I don't see a problem. Using Cache_Lite was not > possible unfortunately, and we don't have anything else. > Yeah, I'm not sure if we have to offer a solution. If Zend_Cache installable via PEAR, we could use that easily. ;) I'm guessing zf2 could fill the gap, or something from the Symfony ecosystem. I wouldn't mind relying on a stable component there. Till