4 ms·

Don't take this as a defense of what is a harebrained idea; but this kind of replacement should be easy to do correctly. You know; in such a way where only real

by swid 3y ago

Don't take this as a defense of what is a harebrained idea; but this kind of replacement should be easy to do correctly. You know; in such a way where only real twitter.com links are changed to x.com. Honestly it is only somewhat surprising to me that no one noticed the error ahead of time. On the one hand, this is the type of mistake I do see in reviews from time to time... usually in the form of a regex that is not anchored to the start of the string, or perhaps it uses a non escaped period which of course means "any character" in regex. On the other hand, it is revealing about the kinds of controls in place that it got through.

notatoad 3y ago
even things that are easy (or maybe, especially things that are easy) can benefit from having a review to make sure you didn't miss something obvious. nothing more dangerous than "oh, that's easy, let's push it straight to prod"
- seanhunter 3y ago
  Example: one of the worst and most costly bugs I ever saw during my time in finance involved a code review [1] with the exchange: "Looks good generally but this will serialize incorrectly if a date is ever negative." <... pause while everyone digests that...> "Why would a date ever be negative?" Noone could think of a reason a date would ever be negative. The resulting code push cost millions in a single day. [1] I was on the call because I was the SRE[2] responsible for doing the release [2] We didn't call ourselves SREs because it was like 2001 and that terminology hadn't caught on yet. We called ourselves "Installmeisters" believe it or not.
  - zestyping 3y ago
    How did it turn out that a date was negative, and how did it cost millions?
    - seanhunter 3y ago
      Iirc it turned out some 20+ year old code was relying on an undocumented/unknown behaviour of dates when they are negative to calculate cashflows on very old perpetual fixed income instruments[1]. If you have a codebase that's large enough and worked on by enough people, for every possible hack/undocumented feature you'll have at least one person who will rely on it in some way. It cost millions, because the serialization corrupted the USD treasury curve when we wrote it to the database, which meant that even though the code was reverted quickly, for a while no derivatives (like literally no derivatives) in the whole of the trading part of a big IB could price (because essentially all derivatives were priced in dollars using some sort of discounted cash flow mechanism which relies on the USD treasury curve). No price means no risk, because how you calculate risk of a derivative is by finding the underlyers, shocking those underlyers a little bit and calculating the new price (essentially an empirical method of finding the derivative of price with respect to each of those things). [1] https://en.wikipedia.org/wiki/Perpetual_bond https://en.wikipedia.org/wiki/Perpetual_bond
perihelions 3y ago
That's a good hypothesis with the non-escaped period! That someone wrote /twitter.com/ for the string substitution, which almost works, and then added a second one like /\w+.twitter.com/ for subdomains, which also seems to work, and would pass simple tests to check if it works or not. Matches everything it's supposed to; rejects most of the things it shouldn't.
- swid 3y ago
  You are right the period would line up with the issue we saw. But also, I can’t help but to think even /\w+\.twitter\.com/ will match unexpected domains like foo.twitter.com.evil.ai. I already gave you the hint it needs to be anchored!

by swid 3y ago

notatoad 3y ago

even things that are easy (or maybe, especially things that are easy) can benefit from having a review to make sure you didn't miss something obvious. nothing more dangerous than "oh, that's easy, let's push it straight to prod"

seanhunter 3y ago
Example: one of the worst and most costly bugs I ever saw during my time in finance involved a code review [1] with the exchange: "Looks good generally but this will serialize incorrectly if a date is ever negative." <... pause while everyone digests that...> "Why would a date ever be negative?" Noone could think of a reason a date would ever be negative. The resulting code push cost millions in a single day. [1] I was on the call because I was the SRE[2] responsible for doing the release [2] We didn't call ourselves SREs because it was like 2001 and that terminology hadn't caught on yet. We called ourselves "Installmeisters" believe it or not.
- zestyping 3y ago
  How did it turn out that a date was negative, and how did it cost millions?
  - seanhunter 3y ago
    Iirc it turned out some 20+ year old code was relying on an undocumented/unknown behaviour of dates when they are negative to calculate cashflows on very old perpetual fixed income instruments[1]. If you have a codebase that's large enough and worked on by enough people, for every possible hack/undocumented feature you'll have at least one person who will rely on it in some way. It cost millions, because the serialization corrupted the USD treasury curve when we wrote it to the database, which meant that even though the code was reverted quickly, for a while no derivatives (like literally no derivatives) in the whole of the trading part of a big IB could price (because essentially all derivatives were priced in dollars using some sort of discounted cash flow mechanism which relies on the USD treasury curve). No price means no risk, because how you calculate risk of a derivative is by finding the underlyers, shocking those underlyers a little bit and calculating the new price (essentially an empirical method of finding the derivative of price with respect to each of those things). [1] https://en.wikipedia.org/wiki/Perpetual_bond https://en.wikipedia.org/wiki/Perpetual_bond

perihelions 3y ago

That's a good hypothesis with the non-escaped period! That someone wrote /twitter.com/ for the string substitution, which almost works, and then added a second one like /\w+.twitter.com/ for subdomains, which also seems to work, and would pass simple tests to check if it works or not. Matches everything it's supposed to; rejects most of the things it shouldn't.

swid 3y ago
You are right the period would line up with the issue we saw. But also, I can’t help but to think even /\w+\.twitter\.com/ will match unexpected domains like foo.twitter.com.evil.ai. I already gave you the hint it needs to be anchored!