7 ms·
Our Git Hash Bug
- theamk 2y agoYAML Norway problem strikes again! I wish YAML wasn't so common in Python ecosystem...
- secondcoming 2y agoor anywhere
- codesnik 2y agoYAML is alright when you create it by hand. Problems start when YAML is templated, and frankly it's just dumb, though common. Templating the source without proper escaping could bite even with JSON.
- xelamonster 2y agoMy last big enterprise job, we did a lot of YAML templating and of course ran into issues like this all the time, eventually though we solved it by requiring defined schemas for all configs and validating those schemas in our pipeline. More overhead but that validation also caught lots of issues aside from the YAML gotchas, it was a decent setup to work with in the end.
- xelamonster 2y agoWhat's a better alternative? This problem is annoying but to me not more annoying than having to read or write JSON, and TOML is terrible with nested configs. If there were an option with the structure of YAML minus the ridiculous string handling I'd switch to it for sure.
- crdrost 2y agoPkl (from Apple land) and Dhall (from Haskell land) both solve some of these pain points as well as some others, especially being more seamless about integrating schema with config. Jsonnet, I haven't used personally but I know people who have raved about it. Ones I know less about include KCL, CUE, and Nickel.
- xelamonster 2y agoI liked what I saw of Pkl, wanted to use it when it was released but it seemed the only parser was JVM-based and it was intended more to be transpiled into other config languages. If that's changing definitely worth revisiting it. Dhall I had to look up, it seems nice as long as the formatting used on website examples is not enforced, because to me that looks like an absolute nightmare but my problems are with the whitespace and not the structure itself.
- zamalek 2y agoI don't believe that executable configuration languages are a good fit at a primary configuration source, I would prefer to have them spit out static config before use. From your list KCL fits that bill (and is a really nice config language).
- steelbrain 2y agoJSONC. https://onury.io/jsonc/ https://onury.io/jsonc/ It can be hard to find an implementation of this if you're working in a not-so-popular language (looking at you Swift) but JSONC has had the best developer experience of anything I've tried so far. It's basically JSON with single and multi line comments and trailing commas.
- Groxx 2y agoJSON5 has a heck of a lot broader support AFAICT. It's basically JavaScript's notation: https://json5.org/ https://json5.org/
- RadiozRadioz 2y agoLooked at the sibling comments. Has everybody collectively forgotten that XML exists? This stuff was solved decades ago, XSDs can check types. TOML nesting is indeed a joke. YAML & JSON "look clean", but try to match up nesting in a large document without help from text editor highlights, then see how easy it is in XML. XML has universal support everywhere, has first-class tooling for everything that's being re-invented in these other languages, but it's not "cool" any more.
- steelbrain 2y agoMaybe you have worked with a different XML than I but I have only terrible memories of working with XML. Starting from the parser inconsistencies (that even led to security vulnerabilities in Apple, see https://blog.siguza.net/psychicpaper/ https://blog.siguza.net/psychicpaper/). On a higher level, the fact that so much different kind of information could exist at each level was nothing but headaches. In YAML, or in JSON, it's pretty straight forward. You have an object, it has children, children have types/values etc. In XML, you have to keep in mind what the tag of the element, what its attributes are, and then what the child elements are, and then whatever the heck CDATA is. I think my fellow posters are looking at the past through nostalgic rose tinted glasses. XML was terrible and I am glad it's not used as widely anymore.
- kdjdjsj 2y agoI too really don't get the hate for XML Features like schema feel very natural in XML whereas JSON schema feels outer worldly; and is seldomly used Being able to clearly distinguish between multiple semantically different types of strings is a blessing And coming back to your example, how is the knowledge in JSON of which attribute is an array, which is an object, which is a string and when order of attributes as well as duplicates matter and when not any easier than XML?
- xelamonster 2y agoMy two biggest issues: 1. No consistency in whether a value is an attribute or a child element, they seem completely interchangeable and redundant. I'm sure there's some nuance I'm missing here but I've worked with my fair share of XML and it has yet to be relevant to me at least. This adds a lot of mostly arbitrary decisions to the design phase, makes it difficult to clearly refer to specific values, and means it's impossible to directly parse XML into an object structure in most languages like you can with JSON. 2. It's awful to read and write. Everything is just a mess of tags and any structure is quickly lost. Line splitting is awkward and often not even attempted (seriously I had coworkers push back when I applied basic formatting to a config file because they claimed it was easier to read with lines 3x the width of anyone's screen). Needing to name the object in both the opening and closing tag wastes space and is absolutely ridiculous when writing by hand (sure editors can kinda handle this now but not perfectly and sometimes I'm using vi over SSH because that's all there is). I really don't understand why so many people are still so attached to it myself.
- ruuda 2y agohttps://rcl-lang.org/ https://rcl-lang.org/
- 8organicbits 2y agoI'm not a fan of that import function.
- imajoredinecon 2y agoProtobuf/thrift?
- eitland 2y agoThe better alternative to yaml are json (particularly json with comments), properties files and xml files. Basically any mainstream configuration syntax. The really big problem with xml wasn't actually the verbosity of xml itself but the fact that it was popular in a time before rails popularized "convention over configuration"'
- bobbylarrybobby 2y agoJson5 is fine. Of the boring config options, it's the best.
- mike_hearn 2y agoI like HOCON, although it's a bit obscure. It's a JSON syntax superset with the same data model, designed for human written config. It doesn't have schema support however you arguably don't need one, because the software that reads the config specifies the type of keys when the value is read and casting takes place then. If the software expects a string, it reads the value as a string, if it expects a number it's parsed as a number and so on. HOCON has hierarchical merging, include files, a more convenient syntax, ability to read environment variables, substitutions, comments and a few other convenience features. In Conveyor, a tool for packaging desktop apps I wrote that uses it, it's also extended so you have "hash-bang includes". Those are includes that specify a program to run instead of a file, the output is then included and parsed at that point. This lets you escape from declarative config to a fully dynamic computation if you need to. You can disable this feature with a command line flag if you don't trust the config you're parsing (and also env var substitution). You can also render the whole config to regular JSON if you need to. I find that set of features to nicely balance config complexity with read/writeability. The main issue is that the main library is not well maintained, and the best implementation is for the JVM. You could give it a C API these days with Native Image but nobody has. Main downside vs yaml is that IDEs and editors can use YAML schemas to give auto-completion whereas they don't do that for HOCON.
- hollerith 2y agoDoes the HOCON parser complain and refuse to proceed if it encounters a comma right before a closing curly brace like the JSON parsers with which I've interacted?
- mike_hearn 2y agoNo no. The syntax is designed for usability, training commas are fine. There's a little tutorial here, with a slider that shows how you can start with json and transform it. The right hand side is valid HOCON at every step: https://conveyor.hydraulic.dev/15.0/configs/hocon/ https://conveyor.hydraulic.dev/15.0/configs/hocon/
- AceJohnny2 2y agoSolution: quote values. Like in JSON.
- IshKebab 2y agoIt's not really a solution though because you only notice you need to do it after it triggers a bug. You need a format that doesn't let you make this mistake in the first place, like [everything except YAML].
- Too 2y agoIf the yaml is generated by code it should be quoted automatically, given that you have a sane yaml library. try this: yaml.dump({"gameServerVersion": "556474378"}, sys.stdout) It will correctly print the quoted version gameServerVersion: '556474378' At least with Python, strings are strings and there is a minimal risk you store a git hash coming from the output of a git subprocess command as a number. The only way this bug could have happened is if doing manual string templating. If you were doing that, equally or worse kind of bugs are waiting for you in any serialization format.
- IshKebab 2y ago> The only way this bug could have happened is if doing manual string templating. I agree you should not do that, but it is common in the YAML world - if you're using YAML you've already decided you don't care if things are reliable. Given that they were using string templating, this would have been caught earlier using a different format. Of course there are other string templating mistakes that other formats would not catch (e.g. forgetting to escape strings), but they are still better than YAML.
- kdjdjsj 2y agoThe problem obviously is using string templating without proper escaping Any format would fail if used like that, be it YAML, JSON, XML...
- unwind 2y agoMeta: I don't understand "RNG" (random number generation, right?) in the title, since the article is about a quoting issue causing a value to be interpreted as a floating-point value in scientific notation, rather than a string. Is that the "random number generation" the title refers to, perhaps? A like puns, but maybe I'm just tired. :)
- Bromeo 2y agoI suppose it's "RNG" if the commit has exactly one 'e' and otherwise only numbers, so that YAML interprets it as scientific notation. I assume otherwise it's always interpreted as a String, as a fallback.
- joshka 2y agoI was also confused by this as a commit id is deterministic. I suspect that the OP may be confused about how git hashes are computed. But thinking deeper, given that the hash is computed from a small amount of entropy (the commit time), plus a seed (the committed code changes), and the previous value (previous commit hash), this is actually fairly similar to the definition of a PRNG. That is to say, it's not an RNG, but for some approximations it's indistinguishable from one.
- richbell 2y ago> I suspect that the OP may be confused about how git hashes are computed. I don't think they are. The use of "RNG" is referring to how a "random" commit sha lead to this bug. Commit SHAs are, for all intents and purposes, random. You could test this against thousands of SHAs and never encounter one that meets the criteria to trigger the bug.
- tmendez 2y agoThis was my thinking, yeah; been playing too much Hearthstone lately so I had RNG on my mind o_o
- bazzargh 2y agoIt's way less than thousands. To get this specific bug - one 'e' in the 7th digit and 9 digits less than 'a' - is 1/16*(10/16)^9 = 0.009095. Just under 1 in 1000. But if the 'e' is in the 2nd,3rd,4th,5th,6th you'd get the same bug, so actually about 6 in 1000. But around 1% of the hashes will be all decimal digits and will parse as numbers ((10/16)^10 = 0.009). That's common enough that I've seen that one cause errors in our code too (somebody had code reading the hash then trying to append it to a string).
- thanatropism 2y agoNestedText + Pydantic.
- Joker_vD 2y agoThat's why I always put several quotes in YAML values, just to be safe: log_config: file: '"/var/log/my_custom_service/mcs.log"'
- AceJohnny2 2y agodoes several actually help or are you just being funny?
- xelamonster 2y agoHeh, maybe I missed the joke too. Thing is all of the problems people generally think of with YAML strings are solved with just one set of quotes so I don't quite get it either way. Maybe this would have some effect on strings that themselves contain quotes? But that's a special case in every single language with quoted strings, not a YAML problem.
- Joker_vD 2y agoSee the sibling comment. But it is a joke, in a sense that this is all very silly. Maybe we shouldn't have been using such a fringe programming language in the first place but who knew 15 years ago that Go would exist?
- AceJohnny2 2y agoOr that Go would be so popular ;) It's always a gamble when using a programming language from with a small community. Will this one get big?
- Joker_vD 2y agoActually, yes. We use yamerl parser for parsing YAML in Erlang and Erlang has both strings and atoms; yamerl generally turns unquoted YAML strings into atoms and double-quoted YAML strings into strings. But nobody else cares about how yamerl parses YAML for Erlang and lots of tool try to re-normalize YAML as they process it; so if we put e.g. config: option: "this_must_be_a_string" into a Helm chart, and then run it through helm, the resulting YAML will be config: option: this_must_be_a_string and when fed to our Erlang application, the option's value will end up being an atom, and it will blow up when we try to e.g. concatenate it with another string. But '"this_must_be_a_string"' resists all such normalization attempts, gets to be passed around as-is, and ends up being parsed as a string with embedded starting and ending quotes but those can be easily stripped away as a global post-pass on a parsed config, before consuming it any further.
- treyd 2y agoI wonder if you could fix this specific case by keeping track of the last game version to be interpreted this way and using that when querying for `Infinity`. That wouldn't be a platonically correct solution but it should work no?
- aftbit 2y agoNice one! I have hit something similar in the past around text input parsing. I bet that `e` notation for entering numbers has caused more bugs than it has actually been used as intended, at least in non-scientific code.
- AceJohnny2 2y agoWe had a similar problem in Python with float(). We have a logfile parser that tries to parse a value and fell back to float(). But if you passed it a hex value that happened to only have decimals and 'e', then Python interpreted it a an exponentiated number, which happened to be larger than what it could natively represent, so returned `Infinity` ! Obviously the bug was on our end with the naive `float()`, which after deliberation should not be used in this generic parser case, but it was interesting tracking down this bug from the DB values all the way up to the parser.
- notamy 2y agoEvery time I read one of these types of stories, I keep telling myself that there needs to be a stricter subset of YAML with far fewer features and things like required string quoting. I feel like it has to exist, but I don't know where it'd be found.
- darknavi 2y agoJSON?
- xelamonster 2y agoJSON but with the structure of YAML, would be nearly perfect to me. JSON was not designed for humans to read and write directly, it happens to be easy enough that it's quite common but it's not exactly a nice experience. Take that base and remove the need to quote key values, enforce indentation for clear structure and to remove the need for braces and commas (the no trailing commas rule in JSON kills me too), and support the markdown-like list syntax alternative and I think really that's all most people want out of YAML anyways. I don't understand or ever want to work with their references or variables or whatever the hell they add, any of that is much better off in code than in config as far as I'm concerned.
- paulv 2y agoThe trailing comma thing gets me nearly every time I edit a json config file. I don't think json supports comments, which makes it a non-starter for a lot of folks.
- xelamonster 2y agoExtra fun: there's now JSONC which does support trailing commas and comments, and projects will use it with a .json extension still because Microsoft would hate for anything to ever be clear and make sense.
- nixme 2y agoStrictYAML https://hitchdev.com/strictyaml/ https://hitchdev.com/strictyaml/
- molticrystal 2y ago1. Anything that in YAML that is not explicitly a number, add quotes to ensure it is treated as a string. 2. Use monotonically increasing variables for versions of software.
- frizlab 2y agoNow you wait for the short hash collision. Why?
- peterldowns 2y agoI've run into this exact YAML problem when trying to use the git hash as a label in a templated kubernetes config file. Same solution — wrap it in quotes — and exact same frustration. Here's to you, and to whoever else runs into this problem next.
- taspeotis 2y agoYelling At My Laptop
- dreamcompiler 2y agoThe solution to this nonsense is and has always been s-expressions. XML reinvented s-expressions but they made a bad job of it and made them much too verbose. I always use s-exprs for problem-free config files but sadly everybody else takes one look and says "but all those parentheses! I'd rather have my strings interpreted as very large numbers, thank you."
- js2 2y agoSo, yes, YAML's implicit typing is a landmine. However, this could've been caught by a validator. Whatever is loading this config file should know that `gameServerVersion` is a string and when it got a number, it should've thrown an error. It could also further validate that it gets a hex string lest something feed it "Infinity".
- henry700 2y agoA number is also a string. Crazy, right?
- js2 2y agoThe following code will only validate when gameServerVersion is a hex string between 9 and 32 characters: #!/usr/bin/env python import sys import re from typing import Annotated from pydantic import BaseModel, AfterValidator, validator from pydantic_yaml import parse_yaml_raw_as def validate_commit_id(s: str) -> str: assert re.search(r"^[0-9a-f]{9,32}$", s), f"{s} is not a commit id" return s CommitId = Annotated[str, AfterValidator(validate_commit_id)] class Model(BaseModel): gameServerVersion: CommitId def main(): text = sys.stdin.read() conf = parse_yaml_raw_as(Model, text) print(conf) if __name__ == "__main__": main() It would have prevented the Git Hash Bug originally described: $ cat conf.yaml gameServerVersion: 556474e378 $ python parse.py < conf.yaml pydantic_core._pydantic_core.ValidationError: 1 validation error for Model gameServerVersion Input should be a valid string [type=string_type, input_value=inf, input_type=float] $ cat conf.yaml gameServerVersion: "556474e378" $ python parse.py < conf.yaml gameServerVersion='556474e378' $ cat conf.yaml gameServerVersion: "Infinity" $ python parse.py < conf.yaml pydantic_core._pydantic_core.ValidationError: 1 validation error for Model gameServerVersion Assertion failed, Infinity is not a commit id [type=assertion_error, input_value='Infinity', input_type=str] It's just good practice to validate things on the way in. Even if they were using JSON as their config file, they should still validate it.
- cratermoon 2y agoGood thing this wasn't a kernel-mode driver for an anti-virus program.
- forrestthewoods 2y agoYAML is egregiously terrible. JSON is slightly better but still bad. I want an interchange format that lets you declare the types on either side: foo : f32 = bin : 0x42280000 Just let me explicitly declare the types and a kajillion problems go away forever.
- deepsun 2y agoThat's because YAML is mainly intended for human interaction. Machine interaction is bungee-strapped later. Same goes for CLI (shell), same for SQL. All the many bugs with them is because they are not for machines. Proper formats for machines don't have those issues (MsgPack, Protobuf, hell even XML/HTML is better).
- plasticeagle 2y agoAlways use the full hash everywhere. Never store the short hash anywhere. It's good only for immediate commandline use, and nothing else.
- saagarjha 2y agoThis doesn't really solve the underlying issue, though.
- augusto-moura 2y agoNot technically, but pratically yes. The chance of having a single letter "e" and everything else a number would be very rare for a full SHA1 hash. I'm not interested in doing the calculations but if anyone wants to do it, it sounds like a cool math problem to solve
- Sophira 2y agoWhile I haven't done the math, you're right that it would need to be an exponential of some kind (assuming this is JavaScript-based). In JavaScript, Number.MAX_VALUE (the biggest number that can be represented without returning Infinity) is a 309-digit number, and Git SHA-1 hashes are only 40 characters long. However, this does mean that the chance will be even smaller than you say. Assuming that all other conditions are met (sole "e", every other character is a digit) * if the sole "e" is in any place from the 2nd to the 37th, the resulting number is almost guaranteed to be returned as Infinity, unless the number happens to begin with "0e". (Leading zeroes, either in the significand or the exponent, would be required for this to not be a guarantee. Any number beginning "0e", on the other hand, will evaluate to 0 and not become Infinity.) * if it's in the 37th place, the resulting number (assuming no leading zeroes) has around a 27% chance to be representable without being returned as Infinity. * if it's in the 38th or 39th place, the resulting number is guaranteed to be small enough that it won't be turned into Infinity. * if it's in the 1st or 40th place, it probably won't be parsed as a number so will not return an error. I'm also not interested in doing the full calculations, and in the end this isn't going to make a significant difference to the chances IMO (except in the "0e" case mentioned above, which seems like it'd make a noticeable difference), but it's interesting to think about the edge cases.
- erik_seaberg 2y agoI once half-assed a tool that summarized deployed builds, importing a CSV to Google Sheets as a team UI. Well, one day the import noticed one of those commit IDs must be a number because it doesn't contain any letters, and then a formula used exponential notation to create a hyperlink that didn't work. Edit: Now that I think about it, it might have been only digits with a leading zero that was dropped.
- xlii 2y agoIt’s easy to point at YAML and shake one’s head: “ya did it again old chap!”. Yet, software engineering is full of foot guns like that. I tend to joke that working in the industry made me lose all the trust I ever had in me. I don’t believe any deployed code unless it can be confirmed by two independent sources and a runtime telemetry. Here, I expected hash collision which is rare, but in my lifetime I’ve seen my share of “it really shouldn’t happen but somehow it happened”. Couple weeks ago I’ve seen author at a random library implementing queue (with ACKs!) for UDP packet transmission. Truly an amazing career for chaos keepers.
- kelnos 2y agoAs soon as I saw the YAML one-line snippet I knew it was a quoting issue. My first thought was that git hash started with a zero, and the parser stripped it off because it was parsing it as a number and not a string. But then I saw "infinity" and was like "wait is the git hash all numbers except there's the letter 'e' in there somewhere"? Ugh. Always quote your YAML strings! I just wrote a configuration format writer for an app I maintain, The C libyaml library (ugh, yes, I have to write this in C) lets you specify the quoting style it will emit for strings... always choose some form of quoting.... always. Also it seems kinda short-sighted for this company to use the short git hash. The short hash is nice for display purposes, but there's always the possibility of a collision (git is smart and will print out a revision that has no current collisions within your repo, but that doesn't mean it won't happen in the future). But for a configuration setting like that, there's really no reason not to use the full hash.
- jeffrallen 2y agoI definitely had "string parsed as number" on my Yaml bugs bingo card. Did not exactly have "floats and hex have the same alphabet and overlapping grammars" but I'll put that on my bingo card for the next Yaml bugs game, which will be... tomorrow. :(
- Jean-Papoulos 2y agoAnd this kids, is why dynamic typing is inherently bad for purposes other then quick scripting/prototyping.
- _ZeD_ 2y agoinfinite is a valid ieee float value. this is a yaml issue, not a dynamic typing one
- wruza 2y agoYaml should be used as a lesson learned, not sure why people still use it as a data format. You should stop even tolerating it, refuse on sight.
- Hendrikto 2y agoJust quote your strings.
- quuxplusone 2y agoSerendipitously, in this other thread sebstefan just mentioned that in PHP, also, md5('240610708') == md5('QNKCDZO') because PHP, also, treats "0e1234" and "0e4567" as numerically equal when you use the weaker-typed '==' instead of the stronger-typed '==='. https://news.ycombinator.com/item?id=41510252#41510814 https://news.ycombinator.com/item?id=41510252#41510814 https://stackoverflow.com/questions/22140204/why-md5240610708-is-equal-to-md5qnkcdzo https://stackoverflow.com/questions/22140204/why-md524061070...
- rurban 2y ago> This value is set dynamically by a TeamCity deployment job. There you have it. Whoever is using teamcity is doomed to fail like this. Not a YAML problem, but a management problem.