1. 28

  2. 12

    There is an important lesson to be learned here about coding standards. This particular bug would have been obvious if either:

    • gotos were forbidden, in which case this could would have been a series of else if blocks
    • Constructs like if requires {} even if they are only one statement.

    IMO, all-out banning goto is too extreme, but enforcing the second one is a good idea and it an be automated as part of a test framework.

    Personally, I’m very against optional syntax, saving a few lines of code is not worth this bug.

    1. 5

      Personally, I’m very against optional syntax, saving a few lines of code is not worth this bug.

      Agreed. Fancy character-saving syntax tricks are about ego and programmer pissing matches. I say: Save that stuff for the obfuscated code contest. If you want to work for me or with me, then your cleverness had better be directed towards furthering our mutual goals. Code should be written for easy readability, not for showing off your knowledge of the ins and outs of the language.

      1. [Comment removed by author]

        1. 1

          Infuriating that a terrible design choice can now masquerade as an philosophical blog post.

      2. 3

        I’m less concerned with Apples coding standards than I am by the fact they don’t appear to have a single negative unit test to check that a wrong cert is detected.

        1. 4

          From the original article:

          A test case could have caught this, but it’s difficult because it’s so deep into the handshake. One needs to write a completely separate TLS stack, with lots of options for sending invalid handshakes. In Chromium we have a patched version of TLSLite to do this sort of thing but I cannot recall that we have a test case for exactly this. (Sounds like I know what my Monday morning involves if not.)

          1. 1

            Good point. I wonder if this is good news. Did anyone discover this by accident and exploit it?

        2. 2

          I spent a while as an embedded engineer, where we made heavy use of goto and braceless ifs, particularly for the construct

          if (condition_a)
          if (condition_b)
          if (condition_c)
          if (condition_d)

          as opposed to long if (condition_a) && (condition_b) ... statements.

          1. 3

            I don’t really see the benefit there. It’s not like the number of symbols in the source means much for the compiled code.

        3. 4

          It is interesting that the bug seems to be the same on iOS and OSX, yet there’s no patch for OSX. Especially since this has gone unnoticed for months, you’d think they would wait to get a Mac patch ready before disclosing.

            1. 3

              This feels like a case study for why code review matters.

              1. 4

                In this case, I think solid code guidelines + mechanizing them would be a better solution.

                1. 1

                  There’s no guarantee that this would have been caught in review. This is a job for a linter.

                  1. 1

                    Does reviewing code guarantee anything? I’m not denying that a linter would be ideal, but I feel like spotting duplication like this is something the human eye is good at doing.

                    1. 1

                      Computers are definitely better though. Taking humans out of everything is the goal isn’t it? :)