• Post Reply Bookmark Topic Watch Topic
  • New Topic

Hungarian Notation discussion  RSS feed

 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Discussion split off from here, as it was getting off-topic]

According to Joel Spolsky, that isn't Hungarian Notation. This is, and it is something rather different. But Spolsky says it's all right.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:According to Joel Spolsky, that isn't Hungarian Notation. This is, and it is something rather different. But Spolsky says it's all right.

Yeah, I noticed that; although the example he cites ('unsafe string') is different again. I have to admit I do quite like the idea of indicating an immutable class; I'm just not quite sure how I'd go about it.

Clever lad, that Joel.

Winston
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:I have to admit I do quite like the idea of indicating an immutable class...

Actually, thinking it through a bit, it's probably only of limited value, since good programming often works to interfaces. I certainly wouldn't want to be indicating it in a variable name.

Sorry Joel...

Winston
 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Having been one of those who rebelled against Systems Hungarian ("What? You want me to go back to coding like it's FORTRAN?"), I found his revelation about what Apps Hungarian is to be rather uplifting.

That said, I feel a bit uneasy about his usName/sName approach to unsafe/safe strings. It does self-document, but it also requires that I never allow two variables to lose their coupling (that is, they need to contain versions of the same value).

Would a better OO approach be to have a class with methods that return unsafe/safe versions of the value stored in instances? Something like this:



With this class, all appearances in Joel's example of sName are replaced with name.s(), all appearances of usName are replaced with name.us(), and two variables that might otherwise become uncoupled are replaced with one instance (called "name") of the RuntimeString class.

Any good?
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stevens Miller wrote:That said, I feel a bit uneasy about his usName/sName approach to unsafe/safe strings. It does self-document, but it also requires that I never allow two variables to lose their coupling (that is, they need to contain versions of the same value).

One thing that's probably worth remembering is that his example appears to relate to JavaScript, where there might be some value to indicating "characteristics" in names (but TBH, I'm not convinced).

Would a better OO approach be to have a class with methods that return unsafe/safe versions of the value stored in instances?

I'm not sure there's really a need. There are two problems I see with Spolsky's "solution":
1. It's aimed at plugging a known vulnerability with a coding convention, and the problem with that is precisely the same as using idioms. What happens when a new version of the language plugs the vulnerability itself? You're left with an arcane coding convention that only has value to historians.
2. Even assuming that there's a similar issue in Java, his solution simply seeks to "highlight" possible problems, when I reckon they should be eliminated. And how do you do that? Off the top of my head, I'd say you should either have two types: String (assuming that's the unsafe variety) and SafeString (its encoded equivalent), or an interface that denotes whether the source or target you're reading or writing to is Safe or not.
I slightly prefer the first because you could then require a source or target to take or return a SafeString; but TBH, I'm no "Webbie", so I don't know if there are any other roadblocks to that approach.

Normally, I find JS quite enlightening; but here I'd say he's over-engineering.

Winston
 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
How about polymorphism such that potentially unsafe calls to a method like this:



Get replaced with calls to a pair of methods like these:



Any future in that approach?
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stevens Miller wrote:How about polymorphism such that potentially unsafe calls to a method like this:
Any future in that approach?

Not sure; although I do think that polymorphism may be involved in a solution. What you've described is an overloaded method which, while I've heard the term "static polymorphism" used to describe it, is definitely not what I envisage when I think of polymorphism. It also doesn't force the programmer to follow procedure.

I suspect to get to a proper solution, we need to go back to the problem. What is it?

This is my description - and feel free to trash it if you don't agree:
1. Strings received from users must not be returned without encoding.
2. The reason they can't be returned is due to a vulnerability called XSS.
3. We cannot simply encode everything we return, since this might also remove tagging that we actually want to be rendered.
My argument against Spolsky's "Hungarian" solution is that it's a band-aid to help prevent returning XSS-exploitable strings, when what we want to do (if at all possible) is to eliminate them.

I think if I was tackling this, the first thing I'd look at is the string itself, since it's the source of our problems. How about this:What does this buy us? A type, specifically designed to combat a known exploit.

It is "unsafe" and "un-encoded" by default, and can be left that way forever if we so choose. Furthermore, it's immutable, so in order to change it in any way, we must return a new XSSString.

However, we also have the option to make an XSSString "safe" by calling xstr.setSafe(true). In that situation, we have to assume that the programmer knows what s/he's doing, and has made a conscious decision to say "this string is safe". You can imagine this being done when a string is made up of encoded and un-encoded parts, and the latter mustn't be encoded before sending.

You could also add "mutators" to aid in that process if you want. I could imagine, for example, a:
public XSSString concat(XSSString... strings) {
method might be very useful.

On its own, it doesn't solve the whole problem, but I think it's a start because you can now write source and/or target classes that use XSStrings rather than Strings (or CharSequence's); and targets can ensure that the ones they submit are safe - if they aren't, they simply send the encode()d version.

Moreover, you now have a type that can be documented, rather than a procedure that has to be learnt.

I'm quite sure there are many other ways of doing it; I just dislike the idea of relying on a coding convention ... especially a Hungarian one.

Winston
 
Sresh Rangi
Ranch Hand
Posts: 54
5
  • Likes 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
GWT does something like this with its SafeHtml classes.

Widgets that take html have methods that take SafeHtml as a parameter instead of a String, and there are only a few possible ways to create an instance of SafeHtml. This forces the programmer to be explicit in escaping and makes it much easier to keep track of where values come from and whether they're safe to output directly. This is more type-safe than using naming conventions.

If you don't need to output html as is, then escaping by default is useful too. Escaping on output is much easier to review than having to trace the lifetime of a value to where it came from and see if it's safe.

Also, it's SafeHtml rather than SafeString, because it's only safe in the context of html. A string that is escaped for html is not safe to use in js/css/xpath etc. Many escaping solutions don't account for different contexts a value can be used in.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sresh Rangi wrote:Also, it's SafeHtml rather than SafeString, because it's only safe in the context of html.

Yes, I can definitely see that. Good point.

Glad to see I'm not totally out in the bleachers on this though, despite my lack of knowledge about "webby" stuff.

Winston
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!