Alex Stapleton | c5fffd3 | 2014-03-18 15:29:00 +0000 | [diff] [blame] | 1 | Submitting patches |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 2 | ================== |
| 3 | |
| 4 | * Always make a new branch for your work. |
| 5 | * Patches should be small to facilitate easier review. `Studies have shown`_ |
| 6 | that review quality falls off as patch size grows. Sometimes this will result |
| 7 | in many small PRs to land a single large feature. |
| 8 | * Larger changes should be discussed on `our mailing list`_ before submission. |
| 9 | * New features and significant bug fixes should be documented in the |
| 10 | :doc:`/changelog`. |
Alex Gaynor | fda9247 | 2014-11-06 17:05:48 -0300 | [diff] [blame] | 11 | * You must have legal permission to distribute any code you contribute to |
| 12 | ``cryptography``, and it must be available under both the BSD and Apache |
| 13 | Software License Version 2.0 licenses. |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 14 | |
| 15 | If you believe you've identified a security issue in ``cryptography``, please |
| 16 | follow the directions on the :doc:`security page </security>`. |
| 17 | |
| 18 | Code |
| 19 | ---- |
| 20 | |
Paul Kehrer | 5b6ce2a | 2014-02-24 20:16:10 -0600 | [diff] [blame] | 21 | When in doubt, refer to :pep:`8` for Python code. You can check if your code |
| 22 | meets our automated requirements by running ``flake8`` against it. If you've |
| 23 | installed the development requirements this will automatically use our |
| 24 | configuration. You can also run the ``tox`` job with ``tox -e pep8``. |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 25 | |
| 26 | `Write comments as complete sentences.`_ |
| 27 | |
Alex Gaynor | ee1d96c | 2014-12-12 10:50:46 -0800 | [diff] [blame] | 28 | Class names which contains acronyms or initialisms should always be |
| 29 | capitalized. A class should be named ``HTTPClient``, not ``HttpClient``. |
| 30 | |
Alex Gaynor | 5951f46 | 2014-11-16 09:08:42 -0800 | [diff] [blame] | 31 | Every code file must start with the boilerplate licensing notice: |
| 32 | |
| 33 | .. code-block:: python |
| 34 | |
| 35 | # This file is dual licensed under the terms of the Apache License, Version |
| 36 | # 2.0, and the BSD License. See the LICENSE file in the root of this repository |
| 37 | # for complete details. |
| 38 | |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 39 | Additionally, every Python code file must contain |
| 40 | |
| 41 | .. code-block:: python |
| 42 | |
| 43 | from __future__ import absolute_import, division, print_function |
| 44 | |
Alex Stapleton | c5fffd3 | 2014-03-18 15:29:00 +0000 | [diff] [blame] | 45 | API considerations |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 46 | ~~~~~~~~~~~~~~~~~~ |
| 47 | |
| 48 | Most projects' APIs are designed with a philosophy of "make easy things easy, |
| 49 | and make hard things possible". One of the perils of writing cryptographic code |
| 50 | is that secure code looks just like insecure code, and its results are almost |
Alex Gaynor | 0c11d04 | 2016-06-02 22:24:22 -0700 | [diff] [blame^] | 51 | always indistinguishable. As a result, ``cryptography`` has, as a design |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 52 | philosophy: "make it hard to do insecure things". Here are a few strategies for |
| 53 | API design that should be both followed, and should inspire other API choices: |
| 54 | |
| 55 | If it is necessary to compare a user provided value with a computed value (for |
| 56 | example, verifying a signature), there should be an API provided that performs |
| 57 | the verification in a secure way (for example, using a constant time |
| 58 | comparison), rather than requiring the user to perform the comparison |
| 59 | themselves. |
| 60 | |
| 61 | If it is incorrect to ignore the result of a method, it should raise an |
| 62 | exception, and not return a boolean ``True``/``False`` flag. For example, a |
| 63 | method to verify a signature should raise ``InvalidSignature``, and not return |
| 64 | whether the signature was valid. |
| 65 | |
| 66 | .. code-block:: python |
| 67 | |
| 68 | # This is bad. |
| 69 | def verify(sig): |
| 70 | # ... |
| 71 | return is_valid |
| 72 | |
| 73 | # Good! |
| 74 | def verify(sig): |
| 75 | # ... |
| 76 | if not is_valid: |
| 77 | raise InvalidSignature |
| 78 | |
| 79 | Every recipe should include a version or algorithmic marker of some sort in its |
| 80 | output in order to allow transparent upgrading of the algorithms in use, as |
| 81 | the algorithms or parameters needed to achieve a given security margin evolve. |
| 82 | |
| 83 | APIs at the :doc:`/hazmat/primitives/index` layer should always take an |
| 84 | explicit backend, APIs at the recipes layer should automatically use the |
| 85 | :func:`~cryptography.hazmat.backends.default_backend`, but optionally allow |
| 86 | specifying a different backend. |
| 87 | |
| 88 | C bindings |
| 89 | ~~~~~~~~~~ |
| 90 | |
Laurens Van Houtven | 0a1d9e1 | 2014-06-23 14:06:16 +0200 | [diff] [blame] | 91 | More information on C bindings can be found in :doc:`the dedicated |
| 92 | section of the documentation <c-bindings>`. |
| 93 | |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 94 | Tests |
| 95 | ----- |
| 96 | |
| 97 | All code changes must be accompanied by unit tests with 100% code coverage (as |
| 98 | measured by the combined metrics across our build matrix). |
| 99 | |
| 100 | When implementing a new primitive or recipe ``cryptography`` requires that you |
Paul Kehrer | 1681a69 | 2014-02-11 23:43:51 -0600 | [diff] [blame] | 101 | provide a set of test vectors. See :doc:`/development/test-vectors` for more |
| 102 | details. |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 103 | |
| 104 | Documentation |
| 105 | ------------- |
| 106 | |
Paul Kehrer | 813b294 | 2014-06-05 12:40:56 -0500 | [diff] [blame] | 107 | All features should be documented with prose in the ``docs`` section. To ensure |
| 108 | it builds and passes `doc8`_ style checks you can run ``tox -e docs``. |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 109 | |
| 110 | Because of the inherent challenges in implementing correct cryptographic |
| 111 | systems, we want to make our documentation point people in the right directions |
| 112 | as much as possible. To that end: |
| 113 | |
| 114 | * When documenting a generic interface, use a strong algorithm in examples. |
| 115 | (e.g. when showing a hashing example, don't use |
| 116 | :class:`~cryptography.hazmat.primitives.hashes.MD5`) |
| 117 | * When giving prescriptive advice, always provide references and supporting |
| 118 | material. |
| 119 | * When there is real disagreement between cryptographic experts, represent both |
| 120 | sides of the argument and describe the trade-offs clearly. |
| 121 | |
| 122 | When documenting a new module in the ``hazmat`` package, its documentation |
| 123 | should begin with the "Hazardous Materials" warning: |
| 124 | |
| 125 | .. code-block:: rest |
| 126 | |
| 127 | .. hazmat:: |
| 128 | |
| 129 | When referring to a hypothetical individual (such as "a person receiving an |
| 130 | encrypted message") use gender neutral pronouns (they/them/their). |
| 131 | |
| 132 | Docstrings are typically only used when writing abstract classes, but should |
| 133 | be written like this if required: |
| 134 | |
| 135 | .. code-block:: python |
| 136 | |
| 137 | def some_function(some_arg): |
| 138 | """ |
| 139 | Does some things. |
| 140 | |
| 141 | :param some_arg: Some argument. |
| 142 | """ |
| 143 | |
| 144 | So, specifically: |
| 145 | |
| 146 | * Always use three double quotes. |
| 147 | * Put the three double quotes on their own line. |
| 148 | * No blank line at the end. |
| 149 | * Use Sphinx parameter/attribute documentation `syntax`_. |
| 150 | |
| 151 | |
| 152 | .. _`Write comments as complete sentences.`: http://nedbatchelder.com/blog/201401/comments_should_be_sentences.html |
| 153 | .. _`syntax`: http://sphinx-doc.org/domains.html#info-field-lists |
Paul Kehrer | 277b701 | 2016-01-01 09:58:53 -0600 | [diff] [blame] | 154 | .. _`Studies have shown`: https://smartbear.com/SmartBear/media/pdfs/11_Best_Practices_for_Peer_Code_Review.pdf |
Paul Kehrer | 0839aa8 | 2014-02-11 22:36:51 -0600 | [diff] [blame] | 155 | .. _`our mailing list`: https://mail.python.org/mailman/listinfo/cryptography-dev |
Paul Kehrer | 277b701 | 2016-01-01 09:58:53 -0600 | [diff] [blame] | 156 | .. _`doc8`: https://github.com/openstack/doc8 |