Monday, March 16, 2009

Need access to that internal? Don’t touch that dial!

The blessing and curse of open source is that the source can be easily changed. 

Internal types and members don’t need to be backward compatible with previous versions. This makes fixing bugs and feature enhancements much easier in future versions of the software.  Making types and members internal cuts down on documentation that must be written, read and maintained.  It protects callers from misusing a type or member and unknowingly introducing a bug, which could be a security problem.  It makes consuming the public area easier since reading over a smaller public API is quicker than reading over an enormous one.

Coding Horror: I’ve read a suggestion that since DotNetOpenId is open source, all types and members within it might as well be made public rather than internal so that it’s easier to use.  Just because something is open source does not mean that we throw design virtues out the window.  The internal scope exists in every statically typed language because it is so useful – not because closed source software somehow needs it to hide their implementation details. 

Coding Horror: I’ve also read emails that casually mention that the writer wanted access to a method in DotNetOpenId that looked useful but was internal.  So they made it public and started using it.  This person did not understand that that method was not designed for use outside of a very specific scenario, and they introduced a significant security hole in their application by misusing it.  It turned out that there was already a public member that did exactly what they wanted.  Had they focused their efforts looking over the public API for the right way to get the feature they needed, as they would have been forced to do if it was a closed source library, they would have found it and never had the security hole.

There is only one valid scenario I’ve seen or heard of where DotNetOpenAuth had its types legitimately made public when they were designed to be internal.  It is for the upcoming OSIS interop tests, and the tests specifically need to do the wrong thing, in order to test that the remote party reacts by rejecting the request.  A good library is designed to make doing the right and secure thing easy to discover and easy to do.  Doing the insecure thing should be difficult or impossible.

In DotNetOpenAuth’s case, doing the insecure thing was impossible in many cases, so yes, types had to be elevated from internal to public in order for these interop tests to work.  But since this is a corner case and not something that any production web site should be doing, these types are only made public in a dedicated branch in source control that will never be merged back into main development, although updates from main can certainly be merged into the osis branch. 

So my plea for you, dear reader, if you see a useful-looking type or member in DotNetOpenAuth or DotNetOpenId, please ask yourself if a public member with the functionality you’re looking for already exists.  And if you don’t think there is, ask the dotnetopenid@googlegroups.com mailing list before you make the change.  Do yourself a favor: don’t touch that [access] dial.

No comments:

Post a Comment