• Raphael (unregistered)

    I except this policy.

  • (nodebb)

    This code is so bad, I can't even joke about it. What even is PolicyUtil? More like inutil if you ask me

  • RLB (unregistered)

    I'd say that TRWTF is JSP, particularly with that awful and redundant JSTL syntax fudged in, but then the alternatives are PHP, ASP, or... what?

  • NewtonIsaacs (unregistered)

    JSP? I don't see any Jackson Structured Programming in there...

  • Martin Tilsted (unregistered) in reply to RLB

    The alternative is to use Java with a Sane template system such as Apache Velocity, or Freemarker.

    JSP was a mistake from the get go.

  • (nodebb) in reply to Martin Tilsted

    I think JSP is quirky for sure, but I've also come to learn that bad code can be written in any language/ framework.

  • Altaree (unregistered)

    The <%-- --%> surrounding the <c:if> means the "if" is commented out.

  • Kleyguerth (github)

    This WTF can be explained by a sequence of minor WTFs:

    At first, the code threw the exception and let it bubble up. Management complained the user should never see error pages.

    OK, catch the exception, mark a flag, display error message in the page. At this point, the policy is fetched inside the try-catch, so if it is not valid, the system does not fetch the policy. Management complained the user should never see any errors.

    Comment out the error, leave a blank page. Now a customer complains the system is broken, all they see is blank page.

    "Oh I see, the code does not fetch the policy if an exception is thrown! Move this out of the exception block and voila!"... OK I don't have a "minor WTF" explanation for this last step, this is major no matter what.

  • Bart Fargo (unregistered) in reply to RLB

    Actually, there is no JSTL used in this code snippet. The author used "scriptlets" instead (Java code embedded directly in the JSP). JSTL is generally preferred to scriptlets for easier readability & maintenance. IMHO, the bigger problem here is the violation of separation of concerns. There's too much business & authorization logic in what should be a simple view. These concerns should be delegated to a controller which can then direct the appropriate view to render the outcome.

  • Little Bobby Tables (unregistered)

    Can't see any major problem with "isValidPolicyNumber" as a name for this method. If it returns false, then one of three things:

    a) the policy number does not exist, therefore, by definition, it is not a valid policy number.

    b) the policy is not valid and/or not active, therefore, by definition, it is not a valid policy number.

    c) the customer is not allowed to see that policy, therefore, by definition, it is not a valid policy number for this customer.

    All of which is an adequate technique for determining whether the policy may be presented on the customer's screen.

    Now if you wanted to do something different with the user experience rather than just telling the customer they can't see that policy (e.g. "you typed the number in wrong" or "your policy has expired" or "this policy does not belong to you, it belongs to customer (customerId) whose password is (customer password)", then yes, maybe you do need to rewrite it a bit, but it might be better security practice not to give too much information away.

  • (nodebb) in reply to Little Bobby Tables

    How about "isValidPolicyNumberForUser"

  • (nodebb) in reply to Little Bobby Tables

    There's something much bigger than that happening. PolicyUtil.getPolicy() passes fiscalCode, which we know to be the "customer identifier". In a normal setting, the key in a database table of policies should be (customer, policy) -- but it seems this method returns a policy without verifying the matching customer. From what I see here, there is either no security check for matching customer and policy, or the method ignores the customer identifier and looks up policies by number only. That's bad. But what of a customer portal UI that allows the end user to type in a policy number? This is TRWTF!

  • Appalled (unregistered)

    Remy, I love that you used the term "bail". I do as well, when writing comments. I've often wondered if there are any language constructs that use a (native, not User Function) keyword "Bail" rather than Return, Exit, Break, etc.

  • jay (unregistered)

    Not the main issue but: I don't see the point of JSTL. When I first read about JSTL, it said "Create complex web pages without any programming!!!" "Without any programming" turned out to mean that you don't need to learn Java with all those complicated IF's and loops and so. Instead, you can just use JSTL with its IF's and loops! Wow, that's so much simpler! Instead of learning a complex programming language, I can just ... learn a complex programming language.

  • Somebody Somewhere (unregistered) in reply to Appalled

    "Bail" really is a fine word, and just has the all the right connotations. You aren't calmly exiting the situation, or returning from it, or even breaking away from it; No, the situation has become so dangerous or intolerable that you're forced to make an emergency exit and get away from the problem as far and fast as possible.

  • (nodebb) in reply to Somebody Somewhere

    Maybe not. "Bail out" is implied here in the sense of jumping out of $VEHICLE before it kills you. But "bail" can also mean to remove all that water that's been leaking into the hold, in which case "bail" is analogous to GarbageCollect. Sort of.

    Then there's "either of the two crosspieces bridging the stumps, which the bowler and fielders try to dislodge with the ball to get the batsman out." . I'm not sure you can verb that meaning.

  • (nodebb) in reply to cellocgw

    Given that we are talking about a serious privacy violation which is a legal matter in many jurisdictions, I take "bail" to be used in the sense of getting out of jail.

  • Anon (unregistered) in reply to Bananafish

    What is wrong with allowing the user to type in a policy number (or more likely using the policy number on the URL)? It is not a secret. It is right there on the customer's er... policy.

    The problem as I see it is setting global flags in the business logic layer, and then trusting the presentation layer to act accordingly when it would be much easier to just clear the output buffer, set the HTTP status code to 404, and terminate any further processing.

    Note: 403 would also be reasonable, but 404 would be a better choice to limit the client's knowledge that other policy numbers even exist.

  • Brian Boorman (google) in reply to Anon

    Because the user could type in /someone else's/ policy number, and if it was valid, see that other person's policy information.

  • John A (unregistered)

    The answer is obvious

    enum Valid {
    TRUE,
    FALSE.
    POLICY_NOT_FOUND
}
  • masonwheeler (github) in reply to Anon

    It's what's known as "forced browsing," a common security vulnerability where a bad server design allows an attacker to view someone else's private data by guessing an ID number because the server software unthinkingly loads and displays the data that corresponds to that ID without first validating that it belongs to the user in question. (Or, in this case, it does validate it but then doesn't properly act on the results!)

  • Tim (unregistered)

    Nothing wrong with control flow via exception, if done right. In fact, it can make the code a lot clearer, and potentially avoids a lot of extra flags and look-ahead checks.

    Perfect example is iterators.

    In Java, you have to check to see if there's another element. Implementors have 2 methods to implement - hasNext() and next(). Callers are allowed to call a combination of hasNext() (any number of times, including zero) and next(). In cases that aren't just delegation to another iterator, the implementation needs to keep track of when next() has been called, and potentially retrieve the next object in either of hasNext() and next(), and keep extra state around so that hasNext() and next() stay in sync.

    In Python, you ask for the next element, and an exception is thrown if the iterator is exhausted. Implementors only need to implement one method - next(), so there's no need to keep extra state around. And in the vast majority of cases the caller doesn't ever see the exception because it's handled by the for loop - or can simply allow it to bubble up to its caller.

  • Decius (unregistered)

    Who wants to take bets on whether isPolicyNumberValid sanitizes the input?

  • Sir Weenie (unregistered) in reply to Tim

    Not at all, my friend, Tim. Flow control using exceptions is insanity absolutely every time.

    Iterators cry out for Optional, not exceptions. Java even has Optional now, so iterators could be redone if desired.

  • Tim (unregistered) in reply to Tim

    Bah - just realised underscore-underscore-next()-underscore-underscore got changed to bolded next() :(

    See Python dunder methods.

  • Anon (unregistered) in reply to Tim

    Or just return a Maybe<T>.

  • (nodebb) in reply to Tim

    Your example is bad. Exceptions are typically more expensive than return values in terms of performance. High quality languages recognize this, and thus use the the word exception; it's a condition that's not common. Python is not a high quality language. In fact, it's a hot steamy pile of WTFiness, which is why they use an expensive exception mechanism for routine conditions such as end of a loop. The result? Python programs are slow as s**t, and frequently use inline C for performance.

  • [email protected] (unregistered) in reply to Mr. TA

    That's a silly comparison. Python is a scripting language. It's readable and maintainable, not super fast. Doing the logic in Python and calling NumPy (C++) to do the heavy lifting is the natural and correct way to do things. As a scripting language it's not bad for speed compared to the others - It'll spank perl, ruby, PHP 6*, bash, while being far more maintainable than any of them. But it's not a speed demon and it's not intended to be.

    The right tool for the right job. If I need a GUI and 'speed' I go for C#, and if I need speed uber alles it's back to C++. If your python is slow, you used a screwdriver where you needed a hammer.

    • PHP7 is amazingly fast and about 2.5x faster than Python 3. So when you use the wrong tool for the job it's less painful.
  • (nodebb) in reply to [email protected]

    Fine, I guess don't use any scripting languages? I'm missing the point here. Like you said, C- based languages are way faster, both managed and especially unmanaged. They have all the features python does, and then some. So why would anyone use python? (Other than "we already know it")

  • UniMatrix69 (unregistered)

    It makes no sense to throw an exception for the validation returning false. The result of the validation query is a boolean true or false regarding the permission a user has to see a particular policy, an error should be thrown ONLY when there is an error (out of bounds, out of range, divide by zero, FILE_NOT_FOUND etc.)

  • Mario (unregistered)

    Ahhhhh finally a good example of Italian WTF.

Leave a comment on “A Policy Exception”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article