canmove, Confirmed users, Bureaucrats and Sysops emeriti
1,334
edits
Writing Code for Chemspill Releases: Difference between revisions
add additional point based on comment a few days ago from jesup (my reformulation, though). (Unfortunately, the good set of comments I got right after my lightning talk are lost to our document retention policy.)
(stub out based on lightning talk I gave last December in Orlando) |
(add additional point based on comment a few days ago from jesup (my reformulation, though). (Unfortunately, the good set of comments I got right after my lightning talk are lost to our document retention policy.)) |
||
| Line 6: | Line 6: | ||
* "probably" isn't good enough. "sounds like it should work" is not good enough. Can you prove it? Go through the things that could possibly go wrong. | * "probably" isn't good enough. "sounds like it should work" is not good enough. Can you prove it? Go through the things that could possibly go wrong. | ||
* change as little as possible (effects, not diffs). Don't change dependencies, and don't change web-exposed behavior more than absolutely required. | * change as little as possible (effects, not diffs). Don't change dependencies, and don't change web-exposed behavior more than absolutely required. | ||
* Defense in depth (multiple fixes that would fix the issue) may be a good idea. But be aware that some of the fixes might be too risky for a chemspill release, and it may be worth getting the others out sooner. Falling back to a controlled crash may be better than leaving part of the security bug unpatched. | |||
When you're reviewing code for a chemspill release (which the author of the code should do too): | When you're reviewing code for a chemspill release (which the author of the code should do too): | ||
* go through all the possible codepaths and understand all of the cases (and do this on all relevant branches, not just mozilla-central) | * go through all the possible codepaths and understand all of the cases (and do this on all relevant branches, not just mozilla-central) | ||
* a code reviewer should probably rewrite the patch from scratch; you might find interesting things in the differences between the patches. | * a code reviewer should probably rewrite the patch from scratch; you might find interesting things in the differences between the patches. | ||