1 Feb 2011 21:16
Re: Freetalk review and issues
> Hashtables and Freetalk version.
>
> f1464e96ea9c0985d066d9f6bb0e36d11b2de639
> - I have already explained that it's bad to use Hashtables. Firstly, they
> synchronize unnecessarily; secondly, they'll go away eventually and are a
> somewhat deprecated API. Put manual null checks in if you need to.
> Finally, my experience suggests that storing maps in db4o is a good idea.
Replaced with maps by commit ccd0cd00bbcb1e2bc5ad95afdb3810bcf82a7ee6
> b2b52cd8a849f27a19b813b939a31fd9d555f018
> - This is an issue here too, as evidenced by your comment about losing
> values! See what I said on WoT.
As explained in my mail w.r.t. WoT, I wont get rid of the Map usage before
final because it is too much work and I have not experienced enough breakage.
> fbb624c5ba110b9a92f695bfb3e46c626ac774b5
> - Adding full Freetalk version may not be sensible/safe long-term - "I'm
> running a broken version, attack me!". It's fine for now though.
On the other hand security by obscurity is not a valid security concept.
> 422ef05d630c5f692046246b105c77016f27f81b
> - you should support other protocols, they are not all gone yet:
> + if (url.substring(0,4) != "http") {
> + url = "http://" + url;
> + }
> + HTMLNode linkNode = new HTMLNode("a", "href",
> "/?_CHECKED_HTTP_="+url, url);
>
> df68cb9a34511b963ebef07fb55fe0e71d942faf
> - Same here. There is a list of protocols in the filter code.
> - You should probably do new URI() as a basic sanity check. Or maybe use
> the GenericReadFilterCallback filter itself to process the URIs?
Fixed, see reply to "[Wot] Freetalk WoT auto-linking"
> Hashtables and Freetalk version.
>
> f1464e96ea9c0985d066d9f6bb0e36d11b2de639
> - I have already explained that it's bad to use Hashtables. Firstly, they
> synchronize unnecessarily; secondly, they'll go away eventually and are a
> somewhat deprecated API. Put manual null checks in if you need to.
> Finally, my experience suggests that storing maps in db4o is a good idea.
Replaced with maps by commit ccd0cd00bbcb1e2bc5ad95afdb3810bcf82a7ee6
> b2b52cd8a849f27a19b813b939a31fd9d555f018
> - This is an issue here too, as evidenced by your comment about losing
> values! See what I said on WoT.
As explained in my mail w.r.t. WoT, I wont get rid of the Map usage before
final because it is too much work and I have not experienced enough breakage.
> fbb624c5ba110b9a92f695bfb3e46c626ac774b5
> - Adding full Freetalk version may not be sensible/safe long-term - "I'm
> running a broken version, attack me!". It's fine for now though.
On the other hand security by obscurity is not a valid security concept.
> 422ef05d630c5f692046246b105c77016f27f81b
> - you should support other protocols, they are not all gone yet:
> + if (url.substring(0,4) != "http") {
> + url = "http://" + url;
> + }
> + HTMLNode linkNode = new HTMLNode("a", "href",
> "/?_CHECKED_HTTP_="+url, url);
>
> df68cb9a34511b963ebef07fb55fe0e71d942faf
> - Same here. There is a list of protocols in the filter code.
> - You should probably do new URI() as a basic sanity check. Or maybe use
> the GenericReadFilterCallback filter itself to process the URIs?
Fixed, see reply to "[Wot] Freetalk WoT auto-linking"
RSS Feed