diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 3ac54a518b5..d4b1dba4340 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -8,6 +8,20 @@ +## Is it a substantial burden for the maintainers to support this? + + + ## Related issue number @@ -21,13 +35,35 @@ - [ ] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. -- [ ] Add a new news fragment into the `CHANGES` folder - * name it `.` for example (588.bugfix) - * if you don't have an `issue_id` change it to the pr id after creating the pr - * ensure type is one of the following: - * `.feature`: Signifying a new feature. - * `.bugfix`: Signifying a bug fix. - * `.doc`: Signifying a documentation improvement. - * `.removal`: Signifying a deprecation or removal of public API. - * `.misc`: A ticket has been closed, but it is not of interest to users. - * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files." +- [ ] Add a new news fragment into the `CHANGES/` folder + * name it `..rst` (e.g. `588.bugfix.rst`) + * if you don't have an issue number, change it to the pull request + number after creating the PR + * `.bugfix`: A bug fix for something the maintainers deemed an + improper undesired behavior that got corrected to match + pre-agreed expectations. + * `.feature`: A new behavior, public APIs. That sort of stuff. + * `.deprecation`: A declaration of future API removals and breaking + changes in behavior. + * `.breaking`: When something public is removed in a breaking way. + Could be deprecated in an earlier release. + * `.doc`: Notable updates to the documentation structure or build + process. + * `.packaging`: Notes for downstreams about unobvious side effects + and tooling. Changes in the test invocation considerations and + runtime assumptions. + * `.contrib`: Stuff that affects the contributor experience. e.g. + Running tests, building the docs, setting up the development + environment. + * `.misc`: Changes that are hard to assign to any of the above + categories. + * Make sure to use full sentences with correct case and punctuation, + for example: + ```rst + Fixed issue with non-ascii contents in doctest text files + -- by :user:`contributor-gh-handle`. + ``` + + Use the past tense or the present tense a non-imperative mood, + referring to what's changed compared to the last released version + of this project. diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index a7d681e806e..295362f03ae 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -391,7 +391,7 @@ jobs: run: | make cythonize - name: Build wheels - uses: pypa/cibuildwheel@v2.16.2 + uses: pypa/cibuildwheel@v2.16.4 env: CIBW_ARCHS_MACOS: x86_64 arm64 universal2 - uses: actions/upload-artifact@v3 @@ -436,8 +436,10 @@ jobs: version_file: aiohttp/__init__.py github_token: ${{ secrets.GITHUB_TOKEN }} dist_dir: dist - fix_issue_regex: "`#(\\d+) `_" - fix_issue_repl: "(#\\1)" + fix_issue_regex: >- + :issue:`(\d+)` + fix_issue_repl: >- + #\1 - name: >- Publish 🐍📦 to PyPI diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index be1fbae1ffc..426da6fe048 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,9 +6,35 @@ repos: language: fail entry: >- Changelog files must be named - ####.(bugfix|feature|removal|doc|misc)(.#)?(.rst)? + ####.( + bugfix + | feature + | deprecation + | breaking + | doc + | packaging + | contrib + | misc + )(.#)?(.rst)? exclude: >- - ^CHANGES/(\.TEMPLATE\.rst|\.gitignore|\d+\.(bugfix|feature|removal|doc|misc)(\.\d+)?(\.rst)?|README\.rst)$ + (?x) + ^ + CHANGES/( + \.gitignore + |(\d+|[0-9a-f]{8}|[0-9a-f]{7}|[0-9a-f]{40})\.( + bugfix + |feature + |deprecation + |breaking + |doc + |packaging + |contrib + |misc + )(\.\d+)?(\.rst)? + |README\.rst + |\.TEMPLATE\.rst + ) + $ files: ^CHANGES/ - id: changelogs-user-role name: Changelog files should use a non-broken :user:`name` role @@ -30,11 +56,11 @@ repos: hooks: - id: yesqa - repo: https://github.com/PyCQA/isort - rev: '5.12.0' + rev: '5.13.2' hooks: - id: isort - repo: https://github.com/psf/black - rev: '23.11.0' + rev: '23.12.1' hooks: - id: black language_version: python3 # Should be a command that runs python @@ -76,7 +102,7 @@ repos: - id: pyupgrade args: ['--py37-plus'] - repo: https://github.com/PyCQA/flake8 - rev: '6.1.0' + rev: '7.0.0' hooks: - id: flake8 additional_dependencies: diff --git a/CHANGES.rst b/CHANGES.rst index df607b329a0..290462927e9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,233 @@ .. towncrier release notes start +3.9.3 (2024-01-29) +================== + +Bug fixes +--------- + +- Fixed backwards compatibility breakage (in 3.9.2) of ``ssl`` parameter when set outside + of ``ClientSession`` (e.g. directly in ``TCPConnector``) -- by :user:`Dreamsorcerer`. + + + *Related issues and pull requests on GitHub:* + :issue:`8097`, :issue:`8098`. + + + + +Miscellaneous internal changes +------------------------------ + +- Improved test suite handling of paths and temp files to consistently use pathlib and pytest fixtures. + + + *Related issues and pull requests on GitHub:* + :issue:`3957`. + + + + +---- + + +3.9.2 (2024-01-28) +================== + +Bug fixes +--------- + +- Fixed server-side websocket connection leak. + + + *Related issues and pull requests on GitHub:* + :issue:`7978`. + + + +- Fixed ``web.FileResponse`` doing blocking I/O in the event loop. + + + *Related issues and pull requests on GitHub:* + :issue:`8012`. + + + +- Fixed double compress when compression enabled and compressed file exists in server file responses. + + + *Related issues and pull requests on GitHub:* + :issue:`8014`. + + + +- Added runtime type check for ``ClientSession`` ``timeout`` parameter. + + + *Related issues and pull requests on GitHub:* + :issue:`8021`. + + + +- Fixed an unhandled exception in the Python HTTP parser on header lines starting with a colon -- by :user:`pajod`. + + Invalid request lines with anything but a dot between the HTTP major and minor version are now rejected. + Invalid header field names containing question mark or slash are now rejected. + Such requests are incompatible with :rfc:`9110#section-5.6.2` and are not known to be of any legitimate use. + + + *Related issues and pull requests on GitHub:* + :issue:`8074`. + + + +- Improved validation of paths for static resources requests to the server -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`8079`. + + + + +Features +-------- + +- Added support for passing :py:data:`True` to ``ssl`` parameter in ``ClientSession`` while + deprecating :py:data:`None` -- by :user:`xiangyan99`. + + + *Related issues and pull requests on GitHub:* + :issue:`7698`. + + + +Breaking changes +---------------- + +- Fixed an unhandled exception in the Python HTTP parser on header lines starting with a colon -- by :user:`pajod`. + + Invalid request lines with anything but a dot between the HTTP major and minor version are now rejected. + Invalid header field names containing question mark or slash are now rejected. + Such requests are incompatible with :rfc:`9110#section-5.6.2` and are not known to be of any legitimate use. + + + *Related issues and pull requests on GitHub:* + :issue:`8074`. + + + + +Improved documentation +---------------------- + +- Fixed examples of ``fallback_charset_resolver`` function in the :doc:`client_advanced` document. -- by :user:`henry0312`. + + + *Related issues and pull requests on GitHub:* + :issue:`7995`. + + + +- The Sphinx setup was updated to avoid showing the empty + changelog draft section in the tagged release documentation + builds on Read The Docs -- by :user:`webknjaz`. + + + *Related issues and pull requests on GitHub:* + :issue:`8067`. + + + + +Packaging updates and notes for downstreams +------------------------------------------- + +- The changelog categorization was made clearer. The + contributors can now mark their fragment files more + accurately -- by :user:`webknjaz`. + + The new category tags are: + + * ``bugfix`` + + * ``feature`` + + * ``deprecation`` + + * ``breaking`` (previously, ``removal``) + + * ``doc`` + + * ``packaging`` + + * ``contrib`` + + * ``misc`` + + + *Related issues and pull requests on GitHub:* + :issue:`8066`. + + + + +Contributor-facing changes +-------------------------- + +- Updated :ref:`contributing/Tests coverage ` section to show how we use ``codecov`` -- by :user:`Dreamsorcerer`. + + + *Related issues and pull requests on GitHub:* + :issue:`7916`. + + + +- The changelog categorization was made clearer. The + contributors can now mark their fragment files more + accurately -- by :user:`webknjaz`. + + The new category tags are: + + * ``bugfix`` + + * ``feature`` + + * ``deprecation`` + + * ``breaking`` (previously, ``removal``) + + * ``doc`` + + * ``packaging`` + + * ``contrib`` + + * ``misc`` + + + *Related issues and pull requests on GitHub:* + :issue:`8066`. + + + + +Miscellaneous internal changes +------------------------------ + +- Replaced all ``tmpdir`` fixtures with ``tmp_path`` in test suite. + + + *Related issues and pull requests on GitHub:* + :issue:`3551`. + + + + +---- + + 3.9.1 (2023-11-26) ================== diff --git a/CHANGES/.TEMPLATE.rst b/CHANGES/.TEMPLATE.rst index a27a1994b53..9334cefd84f 100644 --- a/CHANGES/.TEMPLATE.rst +++ b/CHANGES/.TEMPLATE.rst @@ -11,11 +11,56 @@ {{ underline * definitions[category]['name']|length }} {% if definitions[category]['showcontent'] %} -{% for text, values in sections[section][category].items() %} +{% for text, change_note_refs in sections[section][category].items() %} - {{ text + '\n' }} - {{ values|join(',\n ') + '\n' }} -{% endfor %} + {# + NOTE: Replacing 'e' with 'f' is a hack that prevents Jinja's `int` + NOTE: filter internal implementation from treating the input as an + NOTE: infinite float when it looks like a scientific notation (with a + NOTE: single 'e' char in between digits), raising an `OverflowError`, + NOTE: subsequently. 'f' is still a hex letter so it won't affect the + NOTE: check for whether it's a (short or long) commit hash or not. + Ref: https://github.com/pallets/jinja/issues/1921 + -#} + {%- + set pr_issue_numbers = change_note_refs + | map('lower') + | map('replace', 'e', 'f') + | map('int', default=None) + | select('integer') + | map('string') + | list + -%} + {%- set arbitrary_refs = [] -%} + {%- set commit_refs = [] -%} + {%- with -%} + {%- set commit_ref_candidates = change_note_refs | reject('in', pr_issue_numbers) -%} + {%- for cf in commit_ref_candidates -%} + {%- if cf | length in (7, 8, 40) and cf | int(default=None, base=16) is not none -%} + {%- set _ = commit_refs.append(cf) -%} + {%- else -%} + {%- set _ = arbitrary_refs.append(cf) -%} + {%- endif -%} + {%- endfor -%} + {%- endwith -%} + + {% if pr_issue_numbers -%} + *Related issues and pull requests on GitHub:* + :issue:`{{ pr_issue_numbers | join('`, :issue:`') }}`. + {% endif %} + + {% if commit_refs -%} + *Related commits on GitHub:* + :commit:`{{ commit_refs | join('`, :commit:`') }}`. + {% endif %} + + {% if arbitrary_refs -%} + *Unlinked references:* + {{ arbitrary_refs | join(', ') }}`. + {% endif %} + +{% endfor %} {% else %} - {{ sections[section][category]['']|join(', ') }} @@ -34,3 +79,4 @@ No significant changes. {% endif %} {% endfor %} ---- +{{ '\n' * 2 }} diff --git a/CHANGES/.gitignore b/CHANGES/.gitignore index f935021a8f8..d6409a0dd82 100644 --- a/CHANGES/.gitignore +++ b/CHANGES/.gitignore @@ -1 +1,28 @@ +* +!.TEMPLATE.rst !.gitignore +!README.rst +!*.bugfix +!*.bugfix.rst +!*.bugfix.*.rst +!*.breaking +!*.breaking.rst +!*.breaking.*.rst +!*.contrib +!*.contrib.rst +!*.contrib.*.rst +!*.deprecation +!*.deprecation.rst +!*.deprecation.*.rst +!*.doc +!*.doc.rst +!*.doc.*.rst +!*.feature +!*.feature.rst +!*.feature.*.rst +!*.misc +!*.misc.rst +!*.misc.*.rst +!*.packaging +!*.packaging.rst +!*.packaging.*.rst diff --git a/CHANGES/2835.removal b/CHANGES/2835.breaking.rst similarity index 100% rename from CHANGES/2835.removal rename to CHANGES/2835.breaking.rst diff --git a/CHANGES/2977.removal b/CHANGES/2977.breaking.rst similarity index 100% rename from CHANGES/2977.removal rename to CHANGES/2977.breaking.rst diff --git a/CHANGES/3463.removal b/CHANGES/3463.breaking.rst similarity index 100% rename from CHANGES/3463.removal rename to CHANGES/3463.breaking.rst diff --git a/CHANGES/3538.removal b/CHANGES/3538.breaking.rst similarity index 100% rename from CHANGES/3538.removal rename to CHANGES/3538.breaking.rst diff --git a/CHANGES/3539.removal b/CHANGES/3539.breaking.rst similarity index 100% rename from CHANGES/3539.removal rename to CHANGES/3539.breaking.rst diff --git a/CHANGES/3542.removal b/CHANGES/3542.breaking.rst similarity index 100% rename from CHANGES/3542.removal rename to CHANGES/3542.breaking.rst diff --git a/CHANGES/3547.removal b/CHANGES/3547.breaking.rst similarity index 100% rename from CHANGES/3547.removal rename to CHANGES/3547.breaking.rst diff --git a/CHANGES/3548.removal b/CHANGES/3548.breaking.rst similarity index 100% rename from CHANGES/3548.removal rename to CHANGES/3548.breaking.rst diff --git a/CHANGES/3551.misc b/CHANGES/3551.misc deleted file mode 100644 index 63965c14821..00000000000 --- a/CHANGES/3551.misc +++ /dev/null @@ -1 +0,0 @@ -Replace all tmpdir fixtures with tmp_path in test suite. diff --git a/CHANGES/3580.removal b/CHANGES/3580.breaking.rst similarity index 100% rename from CHANGES/3580.removal rename to CHANGES/3580.breaking.rst diff --git a/CHANGES/3890.removal b/CHANGES/3890.breaking.rst similarity index 100% rename from CHANGES/3890.removal rename to CHANGES/3890.breaking.rst diff --git a/CHANGES/3901.removal b/CHANGES/3901.breaking.rst similarity index 100% rename from CHANGES/3901.removal rename to CHANGES/3901.breaking.rst diff --git a/CHANGES/3929.removal b/CHANGES/3929.breaking.rst similarity index 100% rename from CHANGES/3929.removal rename to CHANGES/3929.breaking.rst diff --git a/CHANGES/3931.removal b/CHANGES/3931.breaking.rst similarity index 100% rename from CHANGES/3931.removal rename to CHANGES/3931.breaking.rst diff --git a/CHANGES/3932.removal b/CHANGES/3932.breaking.rst similarity index 100% rename from CHANGES/3932.removal rename to CHANGES/3932.breaking.rst diff --git a/CHANGES/3933.removal b/CHANGES/3933.breaking.rst similarity index 100% rename from CHANGES/3933.removal rename to CHANGES/3933.breaking.rst diff --git a/CHANGES/3934.removal b/CHANGES/3934.breaking.rst similarity index 100% rename from CHANGES/3934.removal rename to CHANGES/3934.breaking.rst diff --git a/CHANGES/3935.removal b/CHANGES/3935.breaking.rst similarity index 100% rename from CHANGES/3935.removal rename to CHANGES/3935.breaking.rst diff --git a/CHANGES/3939.removal b/CHANGES/3939.breaking.rst similarity index 100% rename from CHANGES/3939.removal rename to CHANGES/3939.breaking.rst diff --git a/CHANGES/3940.removal b/CHANGES/3940.breaking.rst similarity index 100% rename from CHANGES/3940.removal rename to CHANGES/3940.breaking.rst diff --git a/CHANGES/3942.removal b/CHANGES/3942.breaking.rst similarity index 100% rename from CHANGES/3942.removal rename to CHANGES/3942.breaking.rst diff --git a/CHANGES/3945.removal b/CHANGES/3945.breaking.rst similarity index 100% rename from CHANGES/3945.removal rename to CHANGES/3945.breaking.rst diff --git a/CHANGES/3948.removal b/CHANGES/3948.breaking.rst similarity index 100% rename from CHANGES/3948.removal rename to CHANGES/3948.breaking.rst diff --git a/CHANGES/3957.misc b/CHANGES/3957.misc deleted file mode 100644 index b4f9f58edb9..00000000000 --- a/CHANGES/3957.misc +++ /dev/null @@ -1 +0,0 @@ -Improve test suite handling of paths and temp files to consistently use pathlib and pytest fixtures. diff --git a/CHANGES/5278.removal b/CHANGES/5278.breaking.rst similarity index 100% rename from CHANGES/5278.removal rename to CHANGES/5278.breaking.rst diff --git a/CHANGES/5284.removal b/CHANGES/5284.breaking.rst similarity index 100% rename from CHANGES/5284.removal rename to CHANGES/5284.breaking.rst diff --git a/CHANGES/7107.removal b/CHANGES/7107.breaking.rst similarity index 100% rename from CHANGES/7107.removal rename to CHANGES/7107.breaking.rst diff --git a/CHANGES/7698.feature b/CHANGES/7698.feature deleted file mode 100644 index e8c4b3fb452..00000000000 --- a/CHANGES/7698.feature +++ /dev/null @@ -1 +0,0 @@ -Added support for passing `True` to `ssl` while deprecating `None`. -- by :user:`xiangyan99` diff --git a/CHANGES/7978.bugfix b/CHANGES/7978.bugfix deleted file mode 100644 index 3c7dc096ca7..00000000000 --- a/CHANGES/7978.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix websocket connection leak diff --git a/CHANGES/7995.doc b/CHANGES/7995.doc deleted file mode 100644 index 70e3dfa5469..00000000000 --- a/CHANGES/7995.doc +++ /dev/null @@ -1 +0,0 @@ -Fix examples of `fallback_charset_resolver` function in client_advanced documentation. -- by :user:`henry0312` diff --git a/CHANGES/8012.bugfix b/CHANGES/8012.bugfix deleted file mode 100644 index b49a812d9f4..00000000000 --- a/CHANGES/8012.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix :py:class:`~aiohttp.web.FileResponse`. doing blocking I/O in the event loop diff --git a/CHANGES/8014.bugfix b/CHANGES/8014.bugfix deleted file mode 100644 index 681bb5966ae..00000000000 --- a/CHANGES/8014.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix double compress when compression enabled and compressed file exists diff --git a/CHANGES/8021.bugfix b/CHANGES/8021.bugfix deleted file mode 100644 index f43843a587f..00000000000 --- a/CHANGES/8021.bugfix +++ /dev/null @@ -1 +0,0 @@ -Add runtime type check for ``ClientSession`` ``timeout`` parameter. diff --git a/CHANGES/8048.removal b/CHANGES/8048.breaking.rst similarity index 100% rename from CHANGES/8048.removal rename to CHANGES/8048.breaking.rst diff --git a/CHANGES/8099.contrib.rst b/CHANGES/8099.contrib.rst new file mode 100644 index 00000000000..827ecfa5827 --- /dev/null +++ b/CHANGES/8099.contrib.rst @@ -0,0 +1,4 @@ +The pull request template is now asking the contributors to +answer a question about the long-term maintenance challenges +they envision as a result of merging their patches +-- by :user:`webknjaz`. diff --git a/CHANGES/README.rst b/CHANGES/README.rst index 9f619296351..78d8b2f308f 100644 --- a/CHANGES/README.rst +++ b/CHANGES/README.rst @@ -9,7 +9,7 @@ end-users. This is why we enforce collection of the change fragment files in pull requests as per `Towncrier philosophy`_. The idea is that when somebody makes a change, they must record -the bits that would affect end-users only including information +the bits that would affect end-users, only including information that would be useful to them. Then, when the maintainers publish a new release, they'll automatically use these records to compose a change log for the respective version. It is important to @@ -34,8 +34,11 @@ for the users to understand what it means. combined with others, it will be a part of the "news digest" telling the readers **what changed** in a specific version of the library *since the previous version*. You should also use -reStructuredText syntax for highlighting code (inline or block), +*reStructuredText* syntax for highlighting code (inline or block), linking parts of the docs or external sites. +However, you do not need to reference the issue or PR numbers here +as *towncrier* will automatically add a reference to all of the +affected issues when rendering the news file. If you wish to sign your change, feel free to add ``-- by :user:`github-username``` at the end (replace ``github-username`` with your own!). @@ -43,7 +46,7 @@ with your own!). Finally, name your file following the convention that Towncrier understands: it should start with the number of an issue or a PR followed by a dot, then add a patch type, like ``feature``, -``doc``, ``misc`` etc., and add ``.rst`` as a suffix. If you +``doc``, ``contrib`` etc., and add ``.rst`` as a suffix. If you need to add more than one fragment, you may add an optional sequence number (delimited with another period) between the type and the suffix. @@ -51,11 +54,24 @@ and the suffix. In general the name will follow ``..rst`` pattern, where the categories are: -- ``feature``: Any new feature -- ``bugfix``: A bug fix -- ``doc``: A change to the documentation -- ``misc``: Changes internal to the repo like CI, test and build changes -- ``removal``: For deprecations and removals of an existing feature or behavior +- ``bugfix``: A bug fix for something we deemed an improper undesired + behavior that got corrected in the release to match pre-agreed + expectations. +- ``feature``: A new behavior, public APIs. That sort of stuff. +- ``deprecation``: A declaration of future API removals and breaking + changes in behavior. +- ``breaking``: When something public gets removed in a breaking way. + Could be deprecated in an earlier release. +- ``doc``: Notable updates to the documentation structure or build + process. +- ``packaging``: Notes for downstreams about unobvious side effects + and tooling. Changes in the test invocation considerations and + runtime assumptions. +- ``contrib``: Stuff that affects the contributor experience. e.g. + Running tests, building the docs, setting up the development + environment. +- ``misc``: Changes that are hard to assign to any of the above + categories. A pull request may have more than one of these components, for example a code change may introduce a new feature that deprecates an old @@ -70,21 +86,28 @@ File :file:`CHANGES/6045.doc.1.rst`: .. code-block:: rst - Added a ``:user:`` role to Sphinx config -- by :user:`webknjaz` + Added a ``:user:`` role to Sphinx config -- by :user:`webknjaz`. -File :file:`CHANGES/4431.bugfix.rst`: +File :file:`CHANGES/8074.bugfix.rst`: .. code-block:: rst - Fixed HTTP client requests to honor ``no_proxy`` environment - variables -- by :user:`scirelli` + Fixed an unhandled exception in the Python HTTP parser on header + lines starting with a colon -- by :user:`pajod`. + + Invalid request lines with anything but a dot between the HTTP + major and minor version are now rejected. Invalid header field + names containing question mark or slash are now rejected. Such + requests are incompatible with :rfc:`9110#section-5.6.2` and are + not known to be of any legitimate use. File :file:`CHANGES/4594.feature.rst`: .. code-block:: rst Added support for ``ETag`` to :py:class:`~aiohttp.web.FileResponse` - -- by :user:`greshilov`, :user:`serhiy-storchaka` and :user:`asvetlov` + -- by :user:`greshilov`, :user:`serhiy-storchaka` and + :user:`asvetlov`. .. tip:: diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 4e37ee5c186..368908ed130 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -263,6 +263,7 @@ Pankaj Pandey Parag Jain Pau Freixes Paul Colomiets +Paul J. Dorn Paulius Šileikis Paulus Schoutsen Pavel Kamaev diff --git a/aiohttp/connector.py b/aiohttp/connector.py index fafc8caf500..47c32cd1f3e 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -757,7 +757,7 @@ def __init__( timeout_ceil_threshold: float = 5, happy_eyeballs_delay: Optional[float] = 0.25, interleave: Optional[int] = None, - ) -> None: + ): super().__init__( keepalive_timeout=keepalive_timeout, force_close=force_close, diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index 48d014d0045..70804563b31 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -69,12 +69,11 @@ # tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / # "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA # token = 1*tchar -METHRE: Final[Pattern[str]] = re.compile(r"[!#$%&'*+\-.^_`|~0-9A-Za-z]+") -VERSRE: Final[Pattern[str]] = re.compile(r"HTTP/(\d).(\d)") -HDRRE: Final[Pattern[bytes]] = re.compile( - rb"[\x00-\x1F\x7F-\xFF()<>@,;:\[\]={} \t\"\\]" -) -HEXDIGIT = re.compile(rb"[0-9a-fA-F]+") +_TCHAR_SPECIALS: Final[str] = re.escape("!#$%&'*+-.^_`|~") +TOKENRE: Final[Pattern[str]] = re.compile(f"[0-9A-Za-z{_TCHAR_SPECIALS}]+") +VERSRE: Final[Pattern[str]] = re.compile(r"HTTP/(\d)\.(\d)", re.ASCII) +DIGITS: Final[Pattern[str]] = re.compile(r"\d+", re.ASCII) +HEXDIGITS: Final[Pattern[bytes]] = re.compile(rb"[0-9a-fA-F]+") class RawRequestMessage(NamedTuple): @@ -133,6 +132,7 @@ def parse_headers( self, lines: List[bytes] ) -> Tuple["CIMultiDictProxy[str]", RawHeaders]: headers: CIMultiDict[str] = CIMultiDict() + # note: "raw" does not mean inclusion of OWS before/after the field value raw_headers = [] lines_idx = 1 @@ -146,13 +146,14 @@ def parse_headers( except ValueError: raise InvalidHeader(line) from None + if len(bname) == 0: + raise InvalidHeader(bname) + # https://www.rfc-editor.org/rfc/rfc9112.html#section-5.1-2 if {bname[0], bname[-1]} & {32, 9}: # {" ", "\t"} raise InvalidHeader(line) bvalue = bvalue.lstrip(b" \t") - if HDRRE.search(bname): - raise InvalidHeader(bname) if len(bname) > self.max_field_size: raise LineTooLong( "request header name {}".format( @@ -161,6 +162,9 @@ def parse_headers( str(self.max_field_size), str(len(bname)), ) + name = bname.decode("utf-8", "surrogateescape") + if not TOKENRE.fullmatch(name): + raise InvalidHeader(bname) header_length = len(bvalue) @@ -207,7 +211,6 @@ def parse_headers( ) bvalue = bvalue.strip(b" \t") - name = bname.decode("utf-8", "surrogateescape") value = bvalue.decode("utf-8", "surrogateescape") # https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5 @@ -331,7 +334,8 @@ def get_content_length() -> Optional[int]: # Shouldn't allow +/- or other number formats. # https://www.rfc-editor.org/rfc/rfc9110#section-8.6-2 - if not length_hdr.strip(" \t").isdecimal(): + # msg.headers is already stripped of leading/trailing wsp + if not DIGITS.fullmatch(length_hdr): raise InvalidHeader(CONTENT_LENGTH) return int(length_hdr) @@ -559,7 +563,7 @@ def parse_message(self, lines: List[bytes]) -> RawRequestMessage: ) # method - if not METHRE.fullmatch(method): + if not TOKENRE.fullmatch(method): raise BadStatusLine(method) # version @@ -676,8 +680,8 @@ def parse_message(self, lines: List[bytes]) -> RawResponseMessage: raise BadStatusLine(line) version_o = HttpVersion(int(match.group(1)), int(match.group(2))) - # The status code is a three-digit number - if len(status) != 3 or not status.isdecimal(): + # The status code is a three-digit ASCII number, no padding + if len(status) != 3 or not DIGITS.fullmatch(status): raise BadStatusLine(line) status_i = int(status) @@ -818,7 +822,7 @@ def feed_data( if self._lax: # Allow whitespace in lax mode. size_b = size_b.strip() - if not re.fullmatch(HEXDIGIT, size_b): + if not re.fullmatch(HEXDIGITS, size_b): exc = TransferEncodingError( chunk[:pos].decode("ascii", "surrogateescape") ) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index fee4f61a1aa..a05ac6cc3de 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -573,9 +573,14 @@ def url_for( # type: ignore[override] url = url / filename if append_version: + unresolved_path = self._directory.joinpath(filename) try: - filepath = self._directory.joinpath(filename).resolve() - if not self._follow_symlinks: + if self._follow_symlinks: + normalized_path = Path(os.path.normpath(unresolved_path)) + normalized_path.relative_to(self._directory) + filepath = normalized_path.resolve() + else: + filepath = unresolved_path.resolve() filepath.relative_to(self._directory) except (ValueError, FileNotFoundError): # ValueError for case when path point to symlink @@ -640,8 +645,13 @@ async def _handle(self, request: Request) -> StreamResponse: # /static/\\machine_name\c$ or /static/D:\path # where the static dir is totally different raise HTTPForbidden() - filepath = self._directory.joinpath(filename).resolve() - if not self._follow_symlinks: + unresolved_path = self._directory.joinpath(filename) + if self._follow_symlinks: + normalized_path = Path(os.path.normpath(unresolved_path)) + normalized_path.relative_to(self._directory) + filepath = normalized_path.resolve() + else: + filepath = unresolved_path.resolve() filepath.relative_to(self._directory) except (ValueError, FileNotFoundError) as error: # relatively safe diff --git a/docs/_static/img/contributing-cov-comment.svg b/docs/_static/img/contributing-cov-comment.svg new file mode 100644 index 00000000000..c5ba1005641 --- /dev/null +++ b/docs/_static/img/contributing-cov-comment.svg @@ -0,0 +1,55 @@ + + + + + + + + + + + + + + + + + + + + + Hits 31428 31504 +76 + + + + + + + + + + + + - + + + + Misses 632 633 +1 + + + + + + + + + + + + - + + + + Partials 203 205 +2 + + + diff --git a/docs/_static/img/contributing-cov-header.svg b/docs/_static/img/contributing-cov-header.svg new file mode 100644 index 00000000000..f51c8957cd1 --- /dev/null +++ b/docs/_static/img/contributing-cov-header.svg @@ -0,0 +1,15 @@ + + + + + + + + + Codecov + + + + Report + + diff --git a/docs/_static/img/contributing-cov-miss.svg b/docs/_static/img/contributing-cov-miss.svg new file mode 100644 index 00000000000..d431cd0f1fc --- /dev/null +++ b/docs/_static/img/contributing-cov-miss.svg @@ -0,0 +1,709 @@ + + + + + + + + + + + + + + 733 + + + + + + + + + + + + + + 740 + + + + + + + + + + + + + + + + + + async + + + + + + + + + + def + + + + + + + + + + resolve + + + + + ( + + + + + self + + + + + , + + + + + request + + + + + : + + + + + Request + + + + + ) + + + + + + + + + + - + + + + + > + + + + + _Resolve + + + + + : + + + + + + + + + + + 15 + + + + + + + + + + + + + + + + + 734 + + + + + + + + + + + + + + 741 + + + + + + + + + + + + + + + + + + + + + + + if + + + + + + + + + + ( + + + + + + + + + ! + + + + + + + + + + + + + + + + + + + + + + 735 + + + + + + + + + + + + + 742 + + + + + + + + + + + + + + + not + + + + + request + + + + + . + + + + + url + + + + + . + + + + + raw_path + + + + + . + + + + + startswith + + + + + ( + + + + + self + + + + + . + + + + + _prefix2 + + + + + ) + + + + + + + + + + + + + + + + + + 736 + + + + + + + + + + + + + 743 + + + + + + + + + + + + + + + and + + + + + request + + + + + . + + + + + url + + + + + . + + + + + raw_path + + + + + != + + + + + self + + + + + . + + + + + _prefix + + + + + + + + + + + + + + + + + 737 + + + + + + + + + + + + + 744 + + + + + + + + + + + + + + + ) + + + + + : + + + + + + + + + + + + + + + + + + + 738 + + + + + + + + + + + + + + 745 + + + + + + + + + + + + + + + + + + + + + + + return + + + + + + + + + + None + + + + + , + + + + + + + + + + set + + + + + ( + + + + + ) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 739 + + + + + + + + + + + + + + 746 + + + + + + + + + + + + + + + + + + match_info + + + + + = + + + + + + + + + + await + + + + + self + + + + + . + + + + + _app + + + + + . + + + + + router + + + + + . + + + + + resolve + + + + + ( + + + + + request + + + + + ) + + + + + + + + + + + 15 + + + + + + + diff --git a/docs/_static/img/contributing-cov-partial.svg b/docs/_static/img/contributing-cov-partial.svg new file mode 100644 index 00000000000..5eceb26b9eb --- /dev/null +++ b/docs/_static/img/contributing-cov-partial.svg @@ -0,0 +1,268 @@ + + + + + + + + + + + + + + 1001 + + + + + + + + + + + + + + + + + + + + url_part + + + + + = + + + + + request + + + + + . + + + + + rel_url + + + + + . + + + + + raw_path + + + + + + + + + + 15 + + + + + + + + + + + + + + + + + 1002 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + while + + + + + url_part + + + + + : + + + + + + + + + ! + + + + + + + + + + + + + + + + + + + + + + + 1003 + + + + + + + + + + + + + + + + + + + + + + + + + for + + + + + candidate + + + + + in + + + + + resource_index + + + + + . + + + + + get + + + + + ( + + + + + url_part + + + + + , + + + + + + + + + + ( + + + + + ) + + + + + ) + + + + + : + + + + + + + + + + + 15 + + + + + + + diff --git a/docs/changes.rst b/docs/changes.rst index 6a61dfbcc1e..089f67235a1 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,14 +4,17 @@ Changelog ========= -To be included in v\ |release| (if present) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +.. only:: not is_release -.. towncrier-draft-entries:: |release| [UNRELEASED DRAFT] + To be included in v\ |release| (if present) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Released versions -^^^^^^^^^^^^^^^^^ + .. towncrier-draft-entries:: |release| [UNRELEASED DRAFT] + + Released versions + ^^^^^^^^^^^^^^^^^ .. include:: ../CHANGES.rst + :start-after: .. towncrier release notes start .. include:: ../HISTORY.rst diff --git a/docs/conf.py b/docs/conf.py index 172cec0c6fc..5f3037b4bbc 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -17,6 +17,12 @@ from pathlib import Path PROJECT_ROOT_DIR = Path(__file__).parents[1].resolve() +IS_RELEASE_ON_RTD = ( + os.getenv("READTHEDOCS", "False") == "True" + and os.environ["READTHEDOCS_VERSION_TYPE"] == "tag" +) +if IS_RELEASE_ON_RTD: + tags.add("is_release") _docs_path = os.path.dirname(__file__) _version_path = os.path.abspath( diff --git a/docs/contributing.rst b/docs/contributing.rst index cd6a613c576..d35e9cd9c4f 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -158,32 +158,72 @@ Any extra texts (print statements and so on) should be removed. make test-3.10-no-extensions -Tests coverage --------------- +Code coverage +------------- -We are trying hard to have good test coverage; please don't make it worse. +We use *codecov.io* as an indispensable tool for analyzing our coverage +results. Visit https://codecov.io/gh/aio-libs/aiohttp to see coverage +reports for the master branch, history, pull requests etc. -Use: +We'll use an example from a real PR to demonstrate how we use this. +Once the tests run in a PR, you'll see a comment posted by *codecov*. +The most important thing to check here is whether there are any new +missed or partial lines in the report: -.. code-block:: shell +.. image:: _static/img/contributing-cov-comment.svg + +Here, the PR has introduced 1 miss and 2 partials. Now we +click the link in the comment header to open the full report: + +.. image:: _static/img/contributing-cov-header.svg + :alt: Codecov report - $ make cov-dev +Now, if we look through the diff under 'Files changed' we find one of +our partials: -to run test suite and collect coverage information. Once the command -has finished check your coverage at the file that appears in the last -line of the output: -``open file:///.../aiohttp/htmlcov/index.html`` +.. image:: _static/img/contributing-cov-partial.svg + :alt: A while loop with partial coverage. -Please go to the link and make sure that your code change is covered. +In this case, the while loop is never skipped in our tests. This is +probably not worth writing a test for (and may be a situation that is +impossible to trigger anyway), so we leave this alone. +We're still missing a partial and a miss, so we switch to the +'Indirect changes' tab and take a look through the diff there. This +time we find the remaining 2 lines: -The project uses *codecov.io* for storing coverage results. Visit -https://codecov.io/gh/aio-libs/aiohttp for looking on coverage of -master branch, history, pull requests etc. +.. image:: _static/img/contributing-cov-miss.svg + :alt: An if statement that isn't covered anymore. + +After reviewing the PR, we find that this code is no longer needed as +the changes mean that this method will never be called under those +conditions. Thanks to this report, we were able to remove some +redundant code from a performance-critical part of our codebase (this +check would have been run, probably multiple times, for every single +incoming request). + +.. tip:: + Sometimes the diff on *codecov.io* doesn't make sense. This is usually + caused by the branch being out of sync with master. Try merging + master into the branch and it will likely fix the issue. Failing + that, try checking coverage locally as described in the next section. + +Other tools +----------- The browser extension https://docs.codecov.io/docs/browser-extension -is highly recommended for analyzing the coverage just in *Files -Changed* tab on *GitHub Pull Request* review page. +is also a useful tool for analyzing the coverage directly from *Files +Changed* tab on the *GitHub Pull Request* review page. + + +You can also produce coverage reports locally with ``make cov-dev`` +or just adding ``--cov-report=html`` to ``pytest``. + +This will run the test suite and collect coverage information. Once +finished, coverage results can be view by opening: +```console +$ python -m webbrowser -n file://"$(pwd)"/htmlcov/index.html +``` Documentation ------------- @@ -220,40 +260,23 @@ To run spell checker on Linux box you should install it first: $ sudo apt-get install enchant $ pip install sphinxcontrib-spelling + +Preparing a pull request +------------------------ + +When making a pull request, please include a short summary of the changes +and a reference to any issue tickets that the PR is intended to solve. +All PRs with code changes should include tests. All changes should +include a changelog entry. + + Changelog update ---------------- -The ``CHANGES.rst`` file is managed using `towncrier -`_ tool and all non trivial -changes must be accompanied by a news entry. - -To add an entry to the news file, first you need to have created an -issue describing the change you want to make. A Pull Request itself -*may* function as such, but it is preferred to have a dedicated issue -(for example, in case the PR ends up rejected due to code quality -reasons). - -Once you have an issue or pull request, you take the number and you -create a file inside of the ``CHANGES/`` directory named after that -issue number with an extension of ``.removal``, ``.feature``, -``.bugfix``, or ``.doc``. Thus if your issue or PR number is ``1234`` and -this change is fixing a bug, then you would create a file -``CHANGES/1234.bugfix``. PRs can span multiple categories by creating -multiple files (for instance, if you added a feature and -deprecated/removed the old feature at the same time, you would create -``CHANGES/NNNN.feature`` and ``CHANGES/NNNN.removal``). Likewise if a PR touches -multiple issues/PRs you may create a file for each of them with the -exact same contents and *Towncrier* will deduplicate them. - -The contents of this file are *reStructuredText* formatted text that -will be used as the content of the news file entry. You do not need to -reference the issue or PR numbers here as *towncrier* will automatically -add a reference to all of the affected issues when rendering the news -file. - - - -Making a Pull Request +.. include:: ../CHANGES/README.rst + + +Making a pull request --------------------- After finishing all steps make a GitHub_ Pull Request with *master* base branch. diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 145a7f187ca..49beecf5432 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -18,6 +18,7 @@ async asyncio asyncpg asynctest +attrs auth autocalculated autodetection @@ -35,6 +36,7 @@ backports BaseEventLoop basename BasicAuth +behaviour BodyPartReader boolean botocore @@ -66,6 +68,7 @@ CIMultiDict ClientSession cls cmd +codebase codec Codings committer @@ -90,6 +93,7 @@ Cythonize cythonized de deduplicate +defs Dependabot deprecations DER @@ -105,6 +109,7 @@ DNSResolver docstring docstrings DoS +downstreams Dup elasticsearch encodings @@ -314,6 +319,8 @@ Testsuite Tf timestamps TLS +tmp +tmpdir toolbar toplevel towncrier @@ -330,6 +337,7 @@ Unittest unix unsets unstripped +untyped uppercased upstr url diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index 6b1ad199758..5ba3cd33442 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -262,12 +262,22 @@ instead could be enabled with ``show_index`` parameter set to ``True``:: web.static('/prefix', path_to_static_folder, show_index=True) -When a symlink from the static directory is accessed, the server responses to -client with ``HTTP/404 Not Found`` by default. To allow the server to follow -symlinks, parameter ``follow_symlinks`` should be set to ``True``:: +When a symlink that leads outside the static directory is accessed, the server +responds to the client with ``HTTP/404 Not Found`` by default. To allow the server to +follow symlinks that lead outside the static root, the parameter ``follow_symlinks`` +should be set to ``True``:: web.static('/prefix', path_to_static_folder, follow_symlinks=True) +.. caution:: + + Enabling ``follow_symlinks`` can be a security risk, and may lead to + a directory transversal attack. You do NOT need this option to follow symlinks + which point to somewhere else within the static directory, this option is only + used to break out of the security sandbox. Enabling this option is highly + discouraged, and only expected to be used for edge cases in a local + development setting where remote users do not have access to the server. + When you want to enable cache busting, parameter ``append_version`` can be set to ``True`` diff --git a/docs/web_reference.rst b/docs/web_reference.rst index aa2f2f11a69..31898b4c676 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -1785,9 +1785,15 @@ Application and Router by default it's not allowed and HTTP/403 will be returned on directory access. - :param bool follow_symlinks: flag for allowing to follow symlinks from - a directory, by default it's not allowed and - HTTP/404 will be returned on access. + :param bool follow_symlinks: flag for allowing to follow symlinks that lead + outside the static root directory, by default it's not allowed and + HTTP/404 will be returned on access. Enabling ``follow_symlinks`` + can be a security risk, and may lead to a directory transversal attack. + You do NOT need this option to follow symlinks which point to somewhere + else within the static directory, this option is only used to break out + of the security sandbox. Enabling this option is highly discouraged, + and only expected to be used for edge cases in a local development + setting where remote users do not have access to the server. :param bool append_version: flag for adding file version (hash) to the url query string, this value will diff --git a/pyproject.toml b/pyproject.toml index 1f590d002ef..85d7c87eb34 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,12 +5,73 @@ requires = [ build-backend = "setuptools.build_meta" [tool.towncrier] -package = "aiohttp" -filename = "CHANGES.rst" -directory = "CHANGES/" -title_format = "{version} ({project_date})" -template = "CHANGES/.TEMPLATE.rst" -issue_format = "`#{issue} `_" + package = "aiohttp" + filename = "CHANGES.rst" + directory = "CHANGES/" + title_format = "{version} ({project_date})" + template = "CHANGES/.TEMPLATE.rst" + issue_format = "{issue}" + + # NOTE: The types are declared because: + # NOTE: - there is no mechanism to override just the value of + # NOTE: `tool.towncrier.type.misc.showcontent`; + # NOTE: - and, we want to declare extra non-default types for + # NOTE: clarity and flexibility. + + [[tool.towncrier.section]] + path = "" + + [[tool.towncrier.type]] + # Something we deemed an improper undesired behavior that got corrected + # in the release to match pre-agreed expectations. + directory = "bugfix" + name = "Bug fixes" + showcontent = true + + [[tool.towncrier.type]] + # New behaviors, public APIs. That sort of stuff. + directory = "feature" + name = "Features" + showcontent = true + + [[tool.towncrier.type]] + # Declarations of future API removals and breaking changes in behavior. + directory = "deprecation" + name = "Deprecations (removal in next major release)" + showcontent = true + + [[tool.towncrier.type]] + # When something public gets removed in a breaking way. Could be + # deprecated in an earlier release. + directory = "breaking" + name = "Removals and backward incompatible breaking changes" + showcontent = true + + [[tool.towncrier.type]] + # Notable updates to the documentation structure or build process. + directory = "doc" + name = "Improved documentation" + showcontent = true + + [[tool.towncrier.type]] + # Notes for downstreams about unobvious side effects and tooling. Changes + # in the test invocation considerations and runtime assumptions. + directory = "packaging" + name = "Packaging updates and notes for downstreams" + showcontent = true + + [[tool.towncrier.type]] + # Stuff that affects the contributor experience. e.g. Running tests, + # building the docs, setting up the development environment. + directory = "contrib" + name = "Contributor-facing changes" + showcontent = true + + [[tool.towncrier.type]] + # Changes that are hard to assign to any of the above categories. + directory = "misc" + name = "Miscellaneous internal changes" + showcontent = true [tool.cibuildwheel] diff --git a/requirements/constraints.txt b/requirements/constraints.txt index b9d438f1ee1..b313d19add4 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -48,7 +48,7 @@ click==8.1.6 # towncrier # typer # wait-for-it -coverage==7.4.0 +coverage==7.4.1 # via # -r requirements/test.in # pytest-cov @@ -128,7 +128,7 @@ pip-tools==7.3.0 # via -r requirements/dev.in platformdirs==3.10.0 # via virtualenv -pluggy==1.2.0 +pluggy==1.4.0 # via pytest pre-commit==3.5.0 # via -r requirements/lint.in @@ -150,7 +150,7 @@ pyjwt==2.8.0 # pyjwt pyproject-hooks==1.0.0 # via build -pytest==7.4.4 +pytest==8.0.0 # via # -r requirements/lint.in # -r requirements/test.in diff --git a/requirements/dev.txt b/requirements/dev.txt index 0fe855984fc..5c2a71c714e 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -48,7 +48,7 @@ click==8.1.6 # towncrier # typer # wait-for-it -coverage==7.4.0 +coverage==7.4.1 # via # -r requirements/test.in # pytest-cov @@ -125,7 +125,7 @@ pip-tools==7.3.0 # via -r requirements/dev.in platformdirs==3.10.0 # via virtualenv -pluggy==1.2.0 +pluggy==1.4.0 # via pytest pre-commit==3.5.0 # via -r requirements/lint.in @@ -145,7 +145,7 @@ pyjwt==2.8.0 # pyjwt pyproject-hooks==1.0.0 # via build -pytest==7.4.4 +pytest==8.0.0 # via # -r requirements/lint.in # -r requirements/test.in diff --git a/requirements/lint.txt b/requirements/lint.txt index fc3f27057e6..62e26d5e575 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -32,11 +32,11 @@ packaging==23.1 # via pytest platformdirs==3.10.0 # via virtualenv -pluggy==1.2.0 +pluggy==1.4.0 # via pytest pre-commit==3.5.0 # via -r requirements/lint.in -pytest==7.4.4 +pytest==8.0.0 # via -r requirements/lint.in pyyaml==6.0.1 # via pre-commit diff --git a/requirements/test.txt b/requirements/test.txt index c3051b81a45..d4aa14c0de9 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -26,7 +26,7 @@ click==8.1.6 # via # typer # wait-for-it -coverage==7.4.0 +coverage==7.4.1 # via # -r requirements/test.in # pytest-cov @@ -61,7 +61,7 @@ packaging==23.1 # via # gunicorn # pytest -pluggy==1.2.0 +pluggy==1.4.0 # via pytest proxy-py==2.4.4rc4 # via -r requirements/test.in @@ -71,7 +71,7 @@ pycparser==2.21 # via cffi pydantic==1.10.12 # via python-on-whales -pytest==7.4.4 +pytest==8.0.0 # via # -r requirements/test.in # pytest-cov diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index cb3a8ef2a09..5258b05387d 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -3,7 +3,8 @@ import asyncio import re -from typing import Any, List +from contextlib import nullcontext +from typing import Any, Dict, List from unittest import mock from urllib.parse import quote @@ -168,11 +169,27 @@ def test_cve_2023_37276(parser: Any) -> None: parser.feed_data(text) +@pytest.mark.parametrize( + "rfc9110_5_6_2_token_delim", + r'"(),/:;<=>?@[\]{}', +) +def test_bad_header_name(parser: Any, rfc9110_5_6_2_token_delim: str) -> None: + text = f"POST / HTTP/1.1\r\nhead{rfc9110_5_6_2_token_delim}er: val\r\n\r\n".encode() + expectation = pytest.raises(http_exceptions.BadHttpMessage) + if rfc9110_5_6_2_token_delim == ":": + # Inserting colon into header just splits name/value earlier. + expectation = nullcontext() + with expectation: + parser.feed_data(text) + + @pytest.mark.parametrize( "hdr", ( "Content-Length: -5", # https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length "Content-Length: +256", + "Content-Length: \N{superscript one}", + "Content-Length: \N{mathematical double-struck digit one}", "Foo: abc\rdef", # https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5 "Bar: abc\ndef", "Baz: abc\x00def", @@ -265,6 +282,20 @@ def test_parse_headers_longline(parser: Any) -> None: parser.feed_data(text) +def test_parse_unusual_request_line(parser: Any) -> None: + if not isinstance(response, HttpResponseParserPy): + pytest.xfail("Regression test for Py parser. May match C behaviour later.") + text = b"#smol //a HTTP/1.3\r\n\r\n" + messages, upgrade, tail = parser.feed_data(text) + assert len(messages) == 1 + msg, _ = messages[0] + assert msg.compression is None + assert not msg.upgrade + assert msg.method == "#smol" + assert msg.path == "//a" + assert msg.version == (1, 3) + + def test_parse(parser: Any) -> None: text = b"GET /test HTTP/1.1\r\n\r\n" messages, upgrade, tail = parser.feed_data(text) @@ -567,6 +598,45 @@ def test_headers_content_length_err_2(parser: Any) -> None: parser.feed_data(text) +_pad: Dict[bytes, str] = { + b"": "empty", + # not a typo. Python likes triple zero + b"\000": "NUL", + b" ": "SP", + b" ": "SPSP", + # not a typo: both 0xa0 and 0x0a in case of 8-bit fun + b"\n": "LF", + b"\xa0": "NBSP", + b"\t ": "TABSP", +} + + +@pytest.mark.parametrize("hdr", [b"", b"foo"], ids=["name-empty", "with-name"]) +@pytest.mark.parametrize("pad2", _pad.keys(), ids=["post-" + n for n in _pad.values()]) +@pytest.mark.parametrize("pad1", _pad.keys(), ids=["pre-" + n for n in _pad.values()]) +def test_invalid_header_spacing( + parser: Any, pad1: bytes, pad2: bytes, hdr: bytes +) -> None: + text = b"GET /test HTTP/1.1\r\n" b"%s%s%s: value\r\n\r\n" % (pad1, hdr, pad2) + expectation = pytest.raises(http_exceptions.BadHttpMessage) + if pad1 == pad2 == b"" and hdr != b"": + # one entry in param matrix is correct: non-empty name, not padded + expectation = nullcontext() + if pad1 == pad2 == hdr == b"": + if not isinstance(response, HttpResponseParserPy): + pytest.xfail("Regression test for Py parser. May match C behaviour later.") + with expectation: + parser.feed_data(text) + + +def test_empty_header_name(parser: Any) -> None: + if not isinstance(response, HttpResponseParserPy): + pytest.xfail("Regression test for Py parser. May match C behaviour later.") + text = b"GET /test HTTP/1.1\r\n" b":test\r\n\r\n" + with pytest.raises(http_exceptions.BadHttpMessage): + parser.feed_data(text) + + def test_invalid_header(parser: Any) -> None: text = b"GET /test HTTP/1.1\r\n" b"test line\r\n\r\n" with pytest.raises(http_exceptions.BadHttpMessage): @@ -689,6 +759,34 @@ def test_http_request_bad_status_line(parser: Any) -> None: assert r"\n" not in exc_info.value.message +_num: Dict[bytes, str] = { + # dangerous: accepted by Python int() + # unicodedata.category("\U0001D7D9") == 'Nd' + "\N{mathematical double-struck digit one}".encode(): "utf8digit", + # only added for interop tests, refused by Python int() + # unicodedata.category("\U000000B9") == 'No' + "\N{superscript one}".encode(): "utf8number", + "\N{superscript one}".encode("latin-1"): "latin1number", +} + + +@pytest.mark.parametrize("nonascii_digit", _num.keys(), ids=_num.values()) +def test_http_request_bad_status_line_number( + parser: Any, nonascii_digit: bytes +) -> None: + text = b"GET /digit HTTP/1." + nonascii_digit + b"\r\n\r\n" + with pytest.raises(http_exceptions.BadStatusLine): + parser.feed_data(text) + + +def test_http_request_bad_status_line_separator(parser: Any) -> None: + # single code point, old, multibyte NFKC, multibyte NFKD + utf8sep = "\N{arabic ligature sallallahou alayhe wasallam}".encode() + text = b"GET /ligature HTTP/1" + utf8sep + b"1\r\n\r\n" + with pytest.raises(http_exceptions.BadStatusLine): + parser.feed_data(text) + + def test_http_request_bad_status_line_whitespace(parser: Any) -> None: text = b"GET\n/path\fHTTP/1.1\r\n\r\n" with pytest.raises(http_exceptions.BadStatusLine): @@ -710,6 +808,31 @@ def test_http_request_upgrade(parser: Any) -> None: assert tail == b"some raw data" +def test_http_request_parser_utf8_request_line(parser: Any) -> None: + if not isinstance(response, HttpResponseParserPy): + pytest.xfail("Regression test for Py parser. May match C behaviour later.") + messages, upgrade, tail = parser.feed_data( + # note the truncated unicode sequence + b"GET /P\xc3\xbcnktchen\xa0\xef\xb7 HTTP/1.1\r\n" + + # for easier grep: ASCII 0xA0 more commonly known as non-breaking space + # note the leading and trailing spaces + "sTeP: \N{latin small letter sharp s}nek\t\N{no-break space} " + "\r\n\r\n".encode() + ) + msg = messages[0][0] + + assert msg.method == "GET" + assert msg.path == "/Pünktchen\udca0\udcef\udcb7" + assert msg.version == (1, 1) + assert msg.headers == CIMultiDict([("STEP", "ßnek\t\xa0")]) + assert msg.raw_headers == ((b"sTeP", "ßnek\t\xa0".encode()),) + assert not msg.should_close + assert msg.compression is None + assert not msg.upgrade + assert not msg.chunked + assert msg.url.path == URL("/P%C3%BCnktchen\udca0\udcef\udcb7").path + + def test_http_request_parser_utf8(parser: Any) -> None: text = "GET /path HTTP/1.1\r\nx-test:тест\r\n\r\n".encode() messages, upgrade, tail = parser.feed_data(text) @@ -759,9 +882,15 @@ def test_http_request_parser_two_slashes(parser: Any) -> None: assert not msg.chunked -def test_http_request_parser_bad_method(parser: Any) -> None: +@pytest.mark.parametrize( + "rfc9110_5_6_2_token_delim", + [bytes([i]) for i in rb'"(),/:;<=>?@[\]{}'], +) +def test_http_request_parser_bad_method( + parser: Any, rfc9110_5_6_2_token_delim: bytes +) -> None: with pytest.raises(http_exceptions.BadStatusLine): - parser.feed_data(b'G=":<>(e),[T];?" /get HTTP/1.1\r\n\r\n') + parser.feed_data(rfc9110_5_6_2_token_delim + b'ET" /get HTTP/1.1\r\n\r\n') def test_http_request_parser_bad_version(parser: Any) -> None: @@ -979,6 +1108,14 @@ def test_http_response_parser_code_not_int(response: Any) -> None: response.feed_data(b"HTTP/1.1 ttt test\r\n\r\n") +@pytest.mark.parametrize("nonascii_digit", _num.keys(), ids=_num.values()) +def test_http_response_parser_code_not_ascii( + response: Any, nonascii_digit: bytes +) -> None: + with pytest.raises(http_exceptions.BadStatusLine): + response.feed_data(b"HTTP/1.1 20" + nonascii_digit + b" test\r\n\r\n") + + def test_http_request_chunked_payload(parser: Any) -> None: text = b"GET /test HTTP/1.1\r\n" b"transfer-encoding: chunked\r\n\r\n" msg, payload = parser.feed_data(text)[0][0] diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 716fa78fd4a..c3caa4ffcff 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -108,6 +108,97 @@ async def test_follow_symlink( assert (await r.text()) == data +async def test_follow_symlink_directory_traversal( + tmp_path: pathlib.Path, aiohttp_client: AiohttpClient +) -> None: + # Tests that follow_symlinks does not allow directory transversal + data = "private" + + private_file = tmp_path / "private_file" + private_file.write_text(data) + + safe_path = tmp_path / "safe_dir" + safe_path.mkdir() + + app = web.Application() + + # Register global static route: + app.router.add_static("/", str(safe_path), follow_symlinks=True) + client = await aiohttp_client(app) + + await client.start_server() + # We need to use a raw socket to test this, as the client will normalize + # the path before sending it to the server. + reader, writer = await asyncio.open_connection(client.host, client.port) + writer.write(b"GET /../private_file HTTP/1.1\r\n\r\n") + response = await reader.readuntil(b"\r\n\r\n") + assert b"404 Not Found" in response + writer.close() + await writer.wait_closed() + await client.close() + + +async def test_follow_symlink_directory_traversal_after_normalization( + tmp_path: pathlib.Path, aiohttp_client: AiohttpClient +) -> None: + # Tests that follow_symlinks does not allow directory transversal + # after normalization + # + # Directory structure + # |-- secret_dir + # | |-- private_file (should never be accessible) + # | |-- symlink_target_dir + # | |-- symlink_target_file (should be accessible via the my_symlink symlink) + # | |-- sandbox_dir + # | |-- my_symlink -> symlink_target_dir + # + secret_path = tmp_path / "secret_dir" + secret_path.mkdir() + + # This file is below the symlink target and should not be reachable + private_file = secret_path / "private_file" + private_file.write_text("private") + + symlink_target_path = secret_path / "symlink_target_dir" + symlink_target_path.mkdir() + + sandbox_path = symlink_target_path / "sandbox_dir" + sandbox_path.mkdir() + + # This file should be reachable via the symlink + symlink_target_file = symlink_target_path / "symlink_target_file" + symlink_target_file.write_text("readable") + + my_symlink_path = sandbox_path / "my_symlink" + pathlib.Path(str(my_symlink_path)).symlink_to(str(symlink_target_path), True) + + app = web.Application() + + # Register global static route: + app.router.add_static("/", str(sandbox_path), follow_symlinks=True) + client = await aiohttp_client(app) + + await client.start_server() + # We need to use a raw socket to test this, as the client will normalize + # the path before sending it to the server. + reader, writer = await asyncio.open_connection(client.host, client.port) + writer.write(b"GET /my_symlink/../private_file HTTP/1.1\r\n\r\n") + response = await reader.readuntil(b"\r\n\r\n") + assert b"404 Not Found" in response + writer.close() + await writer.wait_closed() + + reader, writer = await asyncio.open_connection(client.host, client.port) + writer.write(b"GET /my_symlink/symlink_target_file HTTP/1.1\r\n\r\n") + response = await reader.readuntil(b"\r\n\r\n") + assert b"200 OK" in response + response = await reader.readuntil(b"readable") + assert response == b"readable" + writer.close() + await writer.wait_closed() + await client.close() + + @pytest.mark.parametrize( "dir_name,filename,data", [ diff --git a/tools/check_changes.py b/tools/check_changes.py index 91db8b92572..6cc4d050cd8 100755 --- a/tools/check_changes.py +++ b/tools/check_changes.py @@ -4,8 +4,21 @@ import sys from pathlib import Path -ALLOWED_SUFFIXES = ["feature", "bugfix", "doc", "removal", "misc"] -PATTERN = re.compile(r"\d+\.(" + "|".join(ALLOWED_SUFFIXES) + r")(\.\d+)?(\.rst)?") +ALLOWED_SUFFIXES = ( + "bugfix", + "feature", + "deprecation", + "breaking", + "doc", + "packaging", + "contrib", + "misc", +) +PATTERN = re.compile( + r"(\d+|[0-9a-f]{8}|[0-9a-f]{7}|[0-9a-f]{40})\.(" + + "|".join(ALLOWED_SUFFIXES) + + r")(\.\d+)?(\.rst)?", +) def get_root(script_path): diff --git a/tools/cleanup_changes.py b/tools/cleanup_changes.py index 673866b8d67..5b931138056 100755 --- a/tools/cleanup_changes.py +++ b/tools/cleanup_changes.py @@ -7,8 +7,21 @@ import subprocess from pathlib import Path -ALLOWED_SUFFIXES = ["feature", "bugfix", "doc", "removal", "misc"] -PATTERN = re.compile(r"(\d+)\.(" + "|".join(ALLOWED_SUFFIXES) + r")(\.\d+)?(\.rst)?") +ALLOWED_SUFFIXES = ( + "bugfix", + "feature", + "deprecation", + "breaking", + "doc", + "packaging", + "contrib", + "misc", +) +PATTERN = re.compile( + r"(\d+|[0-9a-f]{8}|[0-9a-f]{7}|[0-9a-f]{40})\.(" + + "|".join(ALLOWED_SUFFIXES) + + r")(\.\d+)?(\.rst)?", +) def main(): @@ -18,9 +31,10 @@ def main(): for fname in (root / "CHANGES").iterdir(): match = PATTERN.match(fname.name) if match is not None: - num = match.group(1) - tst = f"`#{num} `_" - if tst in changes: + commit_issue_or_pr = match.group(1) + tst_issue_or_pr = f":issue:`{commit_issue_or_pr}`" + tst_commit = f":commit:`{commit_issue_or_pr}`" + if tst_issue_or_pr in changes or tst_commit in changes: subprocess.run(["git", "rm", fname]) delete.append(fname.name) print("Deleted CHANGES records:", " ".join(delete))