Deprecate `MakeFile()` in favor of `StreamReader` and `StreamWriter` #805

julianz- · 2025-12-08T03:14:11Z

We replace the MakeFile factory function with direct class instantiation. The MakeFile function now emits a DeprecationWarning and will be removed in a future release.

The makefile() methods in the SSL adapters and the base Adapter class are also deprecated. These methods are no longer necessary because the adapters now return wrapped sockets that work directly with StreamReader and StreamWriter.

The pyOpenSSL adapter wraps its SSL.Connection objects in a new TLSSocket class. TLSSocket implements a socket interface that handles OpenSSL-specific errors (WantReadError, WantWriteError, etc.). This allows plain StreamReader and StreamWriter to work transparently with pyOpenSSL connections.

Although its makefile is no longer used, the builtin SSL adapter continues to work essentially as before - Python's ssl.SSLSocket already provided a compatible interface that works directly with StreamReader and StreamWriter.

This change eliminates the abstraction leak where HTTPConnection needed to call upon an SSL adapter custom makefile() method. Now, HTTPConnection simply creates StreamReader and StreamWriter instances with whatever socket it receives, whether it's a plain socket, ssl.SSLSocket, or TLSSocket.

This is a step towards unifying the two SSL adapters while simplifying higher-level logic.

codecov · 2025-12-08T03:18:27Z

Codecov Report

❌ Patch coverage is 96.98630% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.49%. Comparing base (c5d5175) to head (fb4f826).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #805      +/-   ##
==========================================
+ Coverage   78.19%   79.49%   +1.30%     
==========================================
  Files          41       42       +1     
  Lines        4788     5102     +314     
  Branches      547      566      +19     
==========================================
+ Hits         3744     4056     +312     
- Misses        905      907       +2     
  Partials      139      139

webknjaz · 2025-12-08T21:41:53Z

cheroot/connections.py

            raise


+def _make_file_object(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE):


I was hoping this trampoline would be made redundant. Why is it necessary? What are the challenges in using the right stream constructors right away?

Ok maybe this is better. I have jettisoned _make_file_object. HTTPConnection now checks server.ssl_adapter to decide whether to use plain StreamReader/StreamWriter or, if there is one, the SSL adapter's makefile() method. pyopenSSL adapter still needs such a method so that it can offer wrapped streaming methods that know how to deal with WantReadError, WantWriteError etc. The builtin adapter is fine relying on the plain streaming classes.

webknjaz · 2025-12-10T16:38:36Z

cheroot/server.py

+        # Check if SSL adapter needs custom wrapped streaming
+        # objects (pyOpenSSL does, builtin doesn't)
+        if server.ssl_adapter is not None and hasattr(
+            server.ssl_adapter,
+            'makefile',
+        ):


This looks like an abstraction leak. We shouldn't make TLS adapters and HTTP connections coupled. Let's think of a better solution. I wonder if this initializer could work with a pre-wrapped socket or something. It doesn't feel right that HTTP layer would care whether TCP's got a TLS tunnel or not.

Also, having a conditionally existing method on adapters seems fragile.

I'm starting to wonder if the server instance would need a makefile() method that would hold the callable selection logic...

This is largely what lead to my earlier TLSSocket approach but I've revisited that implementing a more focussed version. The TLSSocket class now wraps the pyOpenSSL SSL.Connection to handle OpenSSL-specific errors like WantReadError internally. This allows plain StreamReader/StreamWriter to work directly now with pyOpenSSL connections. So we get rid of makefile without any abstraction leaks hopefully. See what you think.

webknjaz · 2025-12-10T16:42:45Z

cheroot/makefile.py

+    """
+    File object attached to a socket object.
+
+    This method is now deprecated: Use StreamReader or StreamWriter directly.


It's a function, not a method.

webknjaz · 2025-12-10T16:43:38Z

cheroot/server.py

    last_used = None

-    def __init__(self, server, sock, makefile=MakeFile):
+    def __init__(self, server, sock):


Removing args from method signatures is a breaking API change. They'd need to be deprecated first. Although, I'm not entirely sure how to handle the constructor split beyond that.

cheroot/ssl/__init__.py

cheroot/ssl/builtin.py

webknjaz · 2025-12-10T23:44:02Z

cheroot/server.py

-            # self.socket is of OpenSSL.SSL.Connection type
-            resp_sock = self.socket._socket
+        # Unwrap to get raw TCP socket
+        resp_sock = self.socket._sock


Does this attr always exist?

Currently handle_no_ssl() will only ever get called when using PyOpenSSLAdapter so yes provided we make sure it's there on that adapter this should work. However, the property mimics the same pattern that ssl.SSLSocket uses and also socket.socket.

Alright, it's probably okay for right now. But I hope that in a follow-up we'll be able to have a better way to reach the underlying socket and won't be accessing the private attributes.

cheroot/ssl/pyopenssl.py

cheroot/test/ssl/test_ssl_pyopenssl.py

We replace the `MakeFile` factory function with direct class instantiation. The `MakeFile` function now emits a `DeprecationWarning` and will be removed in a future release. `makefile()` methods are also deprecated on the adapters and its base class.

psf-chronographer bot added the bot:chronographer:provided label Dec 8, 2025

julianz- force-pushed the remove_makefile_minimal branch from feaecac to 4c42a4d Compare December 8, 2025 03:15

julianz- force-pushed the remove_makefile_minimal branch 2 times, most recently from dad25d2 to d6a334a Compare December 8, 2025 03:33

julianz- mentioned this pull request Dec 8, 2025

Add TLSSocket abstraction for uniform SSL/TLS handling #799

Open

julianz- force-pushed the remove_makefile_minimal branch 2 times, most recently from 6407321 to 58938f6 Compare December 8, 2025 03:47

webknjaz reviewed Dec 8, 2025

View reviewed changes

julianz- force-pushed the remove_makefile_minimal branch 11 times, most recently from 7e162f6 to ea44c0b Compare December 9, 2025 05:40

webknjaz changed the title ~~Deprecated Makefile in favor of StreamReader and StreamWriter.~~ Deprecate MakeFile() in favor of StreamReader and StreamWriter Dec 10, 2025

webknjaz reviewed Dec 10, 2025

View reviewed changes

cheroot/ssl/__init__.py Show resolved Hide resolved

julianz- force-pushed the remove_makefile_minimal branch 2 times, most recently from 1b57f6d to 3ce3eb2 Compare December 10, 2025 23:30

webknjaz reviewed Dec 10, 2025

View reviewed changes

cheroot/ssl/builtin.py Show resolved Hide resolved

webknjaz reviewed Dec 10, 2025

View reviewed changes

julianz- force-pushed the remove_makefile_minimal branch from 3ce3eb2 to 6e99e85 Compare December 10, 2025 23:46

julianz- force-pushed the remove_makefile_minimal branch 5 times, most recently from ff05b9c to 6b422bf Compare December 16, 2025 18:51

github-advanced-security bot found potential problems Dec 16, 2025

View reviewed changes

cheroot/ssl/pyopenssl.py Fixed Show fixed Hide fixed

julianz- force-pushed the remove_makefile_minimal branch 7 times, most recently from 99e08a1 to 2e54b22 Compare December 17, 2025 20:42

github-advanced-security bot found potential problems Dec 17, 2025

View reviewed changes

cheroot/test/ssl/test_ssl_pyopenssl.py Fixed Show fixed Hide fixed

julianz- force-pushed the remove_makefile_minimal branch 14 times, most recently from adb3e86 to 3d15b25 Compare December 18, 2025 21:49

julianz- force-pushed the remove_makefile_minimal branch from 3d15b25 to fb4f826 Compare December 18, 2025 22:36

		raise


		def _make_file_object(sock, mode='r', bufsize=io.DEFAULT_BUFFER_SIZE):

Uh oh!

Deprecate MakeFile() in favor of StreamReader and StreamWriter #805

Are you sure you want to change the base?

Deprecate MakeFile() in favor of StreamReader and StreamWriter #805

Uh oh!

Conversation

julianz- commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julianz- Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Deprecate `MakeFile()` in favor of `StreamReader` and `StreamWriter` #805

Deprecate `MakeFile()` in favor of `StreamReader` and `StreamWriter` #805

julianz- commented Dec 8, 2025 •

edited

Loading

codecov bot commented Dec 8, 2025 •

edited

Loading

julianz- Dec 11, 2025 •

edited

Loading