Skip to content

Avoid closing upstream if its managed by plugins #1518

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

Merged
merged 5 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/test-library.yml
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ jobs:
# max-parallel: 4
matrix:
os:
- macOS-12
- macOS-latest
- Ubuntu-24.04
- Windows-latest
python:
Expand Down Expand Up @@ -697,8 +697,8 @@ jobs:
name: 📊 Node ${{ matrix.node }} @ ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-24.04, windows-latest, macOS-12]
node: ['10.x', '11.x', '12.x']
os: [ubuntu-24.04, windows-latest, macOS-latest]
node: ['20.x']
# max-parallel: 4
fail-fast: false
steps:
Expand All @@ -725,7 +725,7 @@ jobs:

developer:
runs-on: ${{ matrix.os }}
name: 🧑‍💻 👩‍💻 👨‍💻 Developer setup ${{ matrix.node }} @ ${{ matrix.os }}
name: 🧑‍💻 👩‍💻 👨‍💻 Developer setup ${{ matrix.python }} @ ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-24.04, macOS-latest]
Expand Down Expand Up @@ -1049,7 +1049,7 @@ jobs:
uses: pypa/gh-action-pypi-publish@release/v1
with:
password: ${{ secrets.TESTPYPI_API_TOKEN }}
repository_url: https://test.pypi.org/legacy/
repository-url: https://test.pypi.org/legacy/

post-release-repo-update:
name: >-
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ This document describes how contributors can participate and iterate quickly whi

Contributors must start `proxy.py` from source to verify and develop new features / fixes. See `Run proxy.py from command line using repo source` in README.md for usage instructions.

[![WARNING](https://img.shields.io/static/v1?label=MacOS&message=warning&color=red)](https://github.com/abhinavsingh/proxy.py/issues/642#issuecomment-960819271) On `macOS` you must install `Python` using `pyenv`, as `Python` installed via `homebrew` tends to be problematic. See linked thread for more details.
[![WARNING](https://img.shields.io/static/v1?label=MacOS&message=warning&color=red)](https://github.com/abhinavsingh/proxy.py/issues/642) On `macOS` you must install `Python` using `pyenv`, as `Python` installed via `homebrew` tends to be problematic. See linked thread for more details.

### Setup Git Hooks

Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1004,7 +1004,7 @@ Start `proxy.py` as:

`HttpProxyBasePlugin.resolve_dns` callback can also be used to configure `network interface` which must be used as the `source_address` for connection to the upstream server.

See [this thread](https://github.com/abhinavsingh/proxy.py/issues/535#issuecomment-961510862)
See [this thread](https://github.com/abhinavsingh/proxy.py/issues/535)
for more details.

PS: There is no plugin named, but [CustomDnsResolverPlugin](#customdnsresolverplugin)
Expand Down Expand Up @@ -2350,7 +2350,7 @@ Most likely it's a browser integration issue with system keychain.

`curl -v -x username:password@localhost:8899 https://httpbin.org/get`

- See [this thread](https://github.com/abhinavsingh/proxy.py/issues/89#issuecomment-534845710)
- See [this thread](https://github.com/abhinavsingh/proxy.py/issues/89)
for further details.

## Docker image not working on macOS
Expand Down Expand Up @@ -2559,7 +2559,7 @@ Contributors must start `proxy.py` from source to verify and develop new feature
See [Run proxy.py from command line using repo source](#from-command-line-using-repo-source) for details.


[![WARNING](https://img.shields.io/static/v1?label=MacOS&message=warning&color=red)](https://github.com/abhinavsingh/proxy.py/issues/642#issuecomment-960819271) On `macOS`
[![WARNING](https://img.shields.io/static/v1?label=MacOS&message=warning&color=red)](https://github.com/abhinavsingh/proxy.py/issues/642) On `macOS`
you must install `Python` using `pyenv`, as `Python` installed via `homebrew` tends
to be problematic. See linked thread for more details.

Expand Down
13 changes: 6 additions & 7 deletions proxy/http/server/reverse.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
)
self.plugins.append(plugin)
self._upstream_proxy_pass: Optional[str] = None
self._needs_upstream: bool = False

def do_upgrade(self, request: HttpParser) -> bool:
"""Signal web protocol handler to not upgrade websocket requests by default."""
Expand All @@ -72,8 +73,6 @@
raise HttpProtocolException('before_routing closed connection')
request = r

needs_upstream = False

# routes
for plugin in self.plugins:
for route in plugin.routes():
Expand All @@ -84,7 +83,7 @@
self.choice = Url.from_bytes(
random.choice(route[1]),
)
needs_upstream = True
self._needs_upstream = True
break
# Dynamic routes
elif isinstance(route, str):
Expand All @@ -93,7 +92,7 @@
choice = plugin.handle_route(request, pattern)
if isinstance(choice, Url):
self.choice = choice
needs_upstream = True
self._needs_upstream = True

Check warning on line 95 in proxy/http/server/reverse.py

View check run for this annotation

Codecov / codecov/patch

proxy/http/server/reverse.py#L95

Added line #L95 was not covered by tests
self._upstream_proxy_pass = str(self.choice)
elif isinstance(choice, memoryview):
self.client.queue(choice)
Expand All @@ -107,7 +106,7 @@
else:
raise ValueError('Invalid route')

if needs_upstream:
if self._needs_upstream:
assert self.choice and self.choice.hostname
port = (
self.choice.port or DEFAULT_HTTP_PORT
Expand Down Expand Up @@ -150,10 +149,10 @@
)

def on_client_connection_close(self) -> None:
if self.upstream and not self.upstream.closed:
if self._needs_upstream and self.upstream and not self.upstream.closed:
logger.debug('Closing upstream server connection')
self.upstream.close()
self.upstream = None
self.upstream = None

def on_client_data(
self,
Expand Down
Loading