Skip to content

[2.7] bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16448) #16476

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
Oct 8, 2019

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Sep 29, 2019

  • bpo-38216: Allow bypassing input validation

  • bpo-36274: Also allow the URL encoding to be overridden.

  • bpo-38216, bpo-36274: Add tests demonstrating a hook for overriding validation, test demonstrating override encoding, and a test to capture expectation of the interface for the URL.

  • Call with skip_host to avoid tripping on the host checking in the URL.

  • Remove obsolete comment.

  • Make _prepare_path_encoding its own attr.

This makes overriding just that simpler.

Also, don't use the := operator to make backporting easier.

  • Add a news entry.

  • _prepare_path_encoding -> _encode_prepared_path()

  • Once again separate the path validation and request encoding, drastically simplifying the behavior. Drop the guarantee that all processing happens in _prepare_path..
    (cherry picked from commit 7774d78)

Co-authored-by: Jason R. Coombs [email protected]

https://bugs.python.org/issue38216

@jaraco
Copy link
Member Author

jaraco commented Sep 29, 2019

Backporting this change, I observe a couple of things:

  1. The _encode_request call is no longer meaningful because the request construction will implicitly encode the request using the default encoding when the format string is used (request = '%s %s %s'...). In order to keep the code as consistent as possible, I decided to include the call as a pass-through. I'd be just as happy to remove it entirely, but I'll leave that up to the reviewer to decide. It's okay that this functionality is disabled on Python 2 because this functionality was mainly around bpo-36274, which was mainly a concern with the transition to Python 3.
  2. Because _encode_request is no longer meaningful, neither is the test for it, so I've removed that test. Therefore, the meaningful part of this test is that for bpo-38216, adding a (underscore-protected) hook to customize/disable validation.

…alidation and encoding behavior (GH-16448)

* bpo-38216: Allow bypassing input validation

* bpo-36274: Also allow the URL encoding to be overridden.

* bpo-38216, bpo-36274: Add tests demonstrating a hook for overriding validation, test demonstrating override encoding, and a test to capture expectation of the interface for the URL.

* Call with skip_host to avoid tripping on the host checking in the URL.

* Remove obsolete comment.

* Make _prepare_path_encoding its own attr.

This makes overriding just that simpler.

Also, don't use the := operator to make backporting easier.

* Add a news entry.

* _prepare_path_encoding -> _encode_prepared_path()

* Once again separate the path validation and request encoding, drastically simplifying the behavior. Drop the guarantee that all processing happens in _prepare_path..
(cherry picked from commit 7774d78)

Co-authored-by: Jason R. Coombs <[email protected]>
@jaraco
Copy link
Member Author

jaraco commented Sep 30, 2019

As a reminder, this PR is for your consideration. The reported failure cases for bpo-38216 already have workarounds implemented, so there are no known pressing needs for this corrective bugfix. Accepting this change will, however, provide a consistent workaround that applies to other (not-yet-identified) use-cases across all (affected) Python versions, so it seems worthwhile to me.

@benjaminp
Copy link
Contributor

benjaminp commented Oct 8, 2019

Since 2.7 wasn't ever released with the fix for bpo-38216, I'm going to say we should take this just in case.

I'm going to edit your PR to remove the usage of locals(). Otherwise, it looks good. Thanks for doing this.

@benjaminp benjaminp merged commit f5b1abb into python:2.7 Oct 8, 2019
@jaraco jaraco deleted the backport-7774d78-2.7 branch December 10, 2019 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants