Avoid closing upstream if its managed by plugins (#1518)

* Avoid closing upstream if its managed by plugins * Anchor not found due to Sphinx/Github compatibility * Use macos-latest * Just node 12.x. 10 and 11 are too old * node 20.x
abhinavsingh · Feb 12, 2025 · d502349 · d502349
1 parent ad2e107
commit d502349
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 16 deletions.
diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml
@@ -439,7 +439,7 @@ jobs:
       # max-parallel: 4
       matrix:
         os:
-        - macOS-12
+        - macOS-latest
         - Ubuntu-24.04
         - Windows-latest
         python:
@@ -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:
@@ -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]
@@ -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: >-

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -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
 

diff --git a/README.md b/README.md
@@ -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)
@@ -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
@@ -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.
 

diff --git a/proxy/http/server/reverse.py b/proxy/http/server/reverse.py
@@ -46,6 +46,7 @@ def __init__(self, *args: Any, **kwargs: Any):
             )
             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."""
@@ -72,8 +73,6 @@ def handle_request(self, request: HttpParser) -> None:
                 raise HttpProtocolException('before_routing closed connection')
             request = r
 
-        needs_upstream = False
-
         # routes
         for plugin in self.plugins:
             for route in plugin.routes():
@@ -84,7 +83,7 @@ def handle_request(self, request: HttpParser) -> None:
                         self.choice = Url.from_bytes(
                             random.choice(route[1]),
                         )
-                        needs_upstream = True
+                        self._needs_upstream = True
                         break
                 # Dynamic routes
                 elif isinstance(route, str):
@@ -93,7 +92,7 @@ def handle_request(self, request: HttpParser) -> None:
                         choice = plugin.handle_route(request, pattern)
                         if isinstance(choice, Url):
                             self.choice = choice
-                            needs_upstream = True
+                            self._needs_upstream = True
                             self._upstream_proxy_pass = str(self.choice)
                         elif isinstance(choice, memoryview):
                             self.client.queue(choice)
@@ -107,7 +106,7 @@ def handle_request(self, request: HttpParser) -> None:
                 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
@@ -150,10 +149,10 @@ def handle_request(self, request: HttpParser) -> None:
                 )
 
     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,