Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Netrc file is accessed inside exception handler, trigger exception again #7237

Closed
1 task done
pletnes opened this issue Mar 22, 2023 · 6 comments · Fixed by #7378
Closed
1 task done

Netrc file is accessed inside exception handler, trigger exception again #7237

pletnes opened this issue Mar 22, 2023 · 6 comments · Fixed by #7378
Labels

Comments

@pletnes
Copy link

pletnes commented Mar 22, 2023

Describe the bug

Sometimes, the .netrc file is unreadable due to permissions. This is handled in this code:

However, inside the exception handler, this if statement accesses metadata about the path. I assume (but have not yet checked) that netrc_path.is_file() lists metadata of the contents of .netrc's parent directory. This triggers an unchecked exception which, in turn, is not handled by other packages which I'm using.

git blame says the surrounding code is from 5 years ago. I take it this is an edge case, not a new or common issue.

My proposed solution is to warn, but not raise exceptions, if this occurs. This seems to be in spirit with the rest of the code, when the .netrc file is missing.

To Reproduce

I will try to improve on the details here. Preliminary notes:

  • Create a system (docker container?) with .netrc in an unreadable directory
  • Perform `aiohttp

Expected behavior

I expect aiohttp to warn, but not fail, when .netrc is present. I had no idea aiohttp requires a readable .netrc file and the cloud environment I'm working in does not have one.

Logs/tracebacks

They're unreadable. I will try to make a simple reproduction example.

Python Version

$ python --version
Python 3.8.10

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.8.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: None
Author-email: None
License: Apache 2
Location: /local_disk0/.ephemeral_nfs/envs/pythonEnv-bcd1c14b-d199-4b6e-9c19-efcc8df3f6e5/lib/python3.8/site-packages
Requires: aiosignal, async-timeout, attrs, yarl, multidict, frozenlist, charset-normalizer
Required-by: adlfs

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /local_disk0/.ephemeral_nfs/envs/pythonEnv-bcd1c14b-d199-4b6e-9c19-efcc8df3f6e5/lib/python3.8/site-packages
Requires: 
Required-by: yarl, aiohttp

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.8.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /local_disk0/.ephemeral_nfs/envs/pythonEnv-bcd1c14b-d199-4b6e-9c19-efcc8df3f6e5/lib/python3.8/site-packages
Requires: multidict, idna
Required-by: aiohttp

OS

I don't quite know, as I am using a hosted cloud environment. Looks Ubuntu linux based:

$ cat /etc/issue
Ubuntu 20.04.5 LTS \n \l
$ uname -a
Linux 0519-075302-a208ww1k-10-177-253-11 5.4.0-1100-azure #106~18.04.1-Ubuntu SMP Mon Dec 12 21:49:35 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Related component

Client

Additional context

Databricks on MS Azure, inside a notebook, using the adlfs module to access files in Azure Blob Storage.

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@pletnes pletnes added the bug label Mar 22, 2023
@pletnes
Copy link
Author

pletnes commented Mar 22, 2023

Apologies for not having a simple reproduction. I will try to create one.

@pletnes
Copy link
Author

pletnes commented Mar 22, 2023

The start of the traceback is from pathlib.py, where it is trying to stat the file.

File "/local_disk0/.ephemeral_nfs/envs/pythonEnv-bcd1c14b-d199-4b6e-9c19-efcc8df3f6e5/lib/python3.8/site-packages/aiohttp/helpers.py", line 233, in netrc_from_env
    client_logger.warning("Could not read .netrc file: %s", e)
  File "/usr/lib/python3.8/pathlib.py", line 1445, in is_file
    return False
  File "/usr/lib/python3.8/pathlib.py", line 1198, in stat
    return self._accessor.stat(self)
PermissionError: [Errno 13] Permission denied: '/root/.netrc'

@pletnes
Copy link
Author

pletnes commented Mar 22, 2023

If I add this line to my python code, I do not encounter the bug. This is because the left side of the or operator evaluates to True, meaning netrc_path.is_file() never runs.

  os.environ["NETRC"] = 'workaround_for_aiohttp_bug'

As a user, this is a viable workaround if you encounter this issue.

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Mar 22, 2023

Sounds reasonable, if you have the time to create a test in a PR (and ideally a fix), that'd be great. In the test, you can probably just monkeypatch the open() (or whatever call is involved) and have it raise the exception.

@jgosmann
Copy link
Contributor

We ran into the same problem today. I might look into providing a PR as it should be a fairly trivial fix. Though, I'm not quite sure if something nicer than nesting try/catch blocks can be done there. 🤔

jgosmann added a commit to jgosmann/aiohttp that referenced this issue Jul 18, 2023
If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None.
Dreamsorcerer pushed a commit that referenced this issue Jul 18, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue #7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes #7237
jgosmann added a commit to jgosmann/aiohttp that referenced this issue Jul 20, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue aio-libs#7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes aio-libs#7237

(cherry picked from commit 0d2e43b)

# Conflicts:
#	aiohttp/helpers.py
jgosmann added a commit to jgosmann/aiohttp that referenced this issue Jul 20, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue aio-libs#7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes aio-libs#7237

(cherry picked from commit 0d2e43b)

# Conflicts:
#	CONTRIBUTORS.txt
#	aiohttp/helpers.py
#	tests/test_helpers.py
jgosmann added a commit to jgosmann/aiohttp that referenced this issue Jul 20, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue aio-libs#7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes aio-libs#7237

(cherry picked from commit 0d2e43b)

# Conflicts:
#	CONTRIBUTORS.txt
#	aiohttp/helpers.py
#	tests/test_helpers.py
Dreamsorcerer pushed a commit that referenced this issue Jul 22, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue #7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes #7237

Backport of #7378 

(cherry picked from commit 0d2e43b)


## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [x] 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.
- [x] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` 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."
Dreamsorcerer pushed a commit that referenced this issue Jul 22, 2023
## What do these changes do?

If no NETRC environment variable is provided and the .netrc path cannot
be accessed due to missing permission, a PermissionError was raised
instead of returning None. See issue #7237. This PR fixes the issue.

If the changes look good, I can also prepare backports.

## Are there changes in behavior for the user?

If the .netrc cannot be accessed due to a permission problem (and the
`NETRC` environment variable is unset), no `PermissionError` will be
raised. Instead it will be silently ignored.

## Related issue number

Fixes #7237

Backport of #7378

(cherry picked from commit 0d2e43b)

## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [x] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [x] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` 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."
@pletnes
Copy link
Author

pletnes commented Jul 31, 2023

Thanks for fixing @jgosmann 🥇

renovate bot added a commit to allenporter/pyrainbird that referenced this issue Oct 9, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [aiohttp](https://togithub.com/aio-libs/aiohttp) | `==3.8.5` ->
`==3.8.6` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/aiohttp/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/aiohttp/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/aiohttp/3.8.5/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/aiohttp/3.8.5/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>aio-libs/aiohttp (aiohttp)</summary>

###
[`v3.8.6`](https://togithub.com/aio-libs/aiohttp/blob/HEAD/CHANGES.rst#386-2023-10-07)

[Compare
Source](https://togithub.com/aio-libs/aiohttp/compare/v3.8.5...v3.8.6)

\==================

## Security bugfixes

- Upgraded the vendored copy of llhttp\_ to v9.1.3 -- by
:user:`Dreamsorcerer`

    Thanks to :user:`kenballus` for reporting this, see

GHSA-pjjw-qhg8-p2p9.

    .. \_llhttp: https://llhttp.org

    `#&#8203;7647 <https://github.com/aio-libs/aiohttp/issues/7647>`\_

- Updated Python parser to comply with RFCs 9110/9112 -- by
:user:`Dreamorcerer`

    Thanks to :user:`kenballus` for reporting this, see

GHSA-gfw2-4jvh-wgfg.

    `#&#8203;7663 <https://github.com/aio-libs/aiohttp/issues/7663>`\_

## Deprecation

- Added `fallback_charset_resolver` parameter in `ClientSession` to
allow a user-supplied
    character set detection function.

Character set detection will no longer be included in 3.9 as a default.
If this feature is needed,
please use `fallback_charset_resolver
<https://docs.aiohttp.org/en/stable/client_advanced.html#character-set-detection>`\_.

    `#&#8203;7561 <https://github.com/aio-libs/aiohttp/issues/7561>`\_

## Features

- Enabled lenient response parsing for more flexible parsing in the
client
(this should resolve some regressions when dealing with badly formatted
HTTP responses). -- by :user:`Dreamsorcerer`

    `#&#8203;7490 <https://github.com/aio-libs/aiohttp/issues/7490>`\_

## Bugfixes

- Fixed `PermissionError` when `.netrc` is unreadable due to
permissions.

    `#&#8203;7237 <https://github.com/aio-libs/aiohttp/issues/7237>`\_

- Fixed output of parsing errors pointing to a `\n`. -- by
:user:`Dreamsorcerer`

    `#&#8203;7468 <https://github.com/aio-libs/aiohttp/issues/7468>`\_

-   Fixed `GunicornWebWorker` max_requests_jitter not working.

    `#&#8203;7518 <https://github.com/aio-libs/aiohttp/issues/7518>`\_

- Fixed sorting in `filter_cookies` to use cookie with longest path. --
by :user:`marq24`.

    `#&#8203;7577 <https://github.com/aio-libs/aiohttp/issues/7577>`\_

- Fixed display of `BadStatusLine` messages from llhttp\_. -- by
:user:`Dreamsorcerer`

    `#&#8203;7651 <https://github.com/aio-libs/aiohttp/issues/7651>`\_

***

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/pyrainbird).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjMiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjMiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants