-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
{testsdk} Add ContentLengthProcessor
to fix Content-Length
header
#20668
Conversation
ContentLengthProcessor
to fix Content-Length
headerContentLengthProcessor
to fix Content-Length
header
14102cd
to
b359a19
Compare
@@ -159,7 +162,7 @@ def _process_request_recording(self, request): | |||
return None | |||
|
|||
if self.in_recording: | |||
for processor in self.recording_processors: | |||
for processor in self.recording_processors + self.recording_post_processors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't add ContentLengthProcessor
here
azure-cli/src/azure-cli-testsdk/azure/cli/testsdk/base.py
Lines 89 to 97 in 30216d0
default_recording_processors = [ | |
SubscriptionRecordingProcessor(MOCKED_SUBSCRIPTION_ID), | |
AADAuthRequestFilter(), | |
LargeRequestBodyProcessor(), | |
LargeResponseBodyProcessor(), | |
DeploymentNameReplacer(), | |
RequestUrlNormalizer(), | |
self.name_replacer | |
] + self._processors_to_reset |
because AbstractPreparer
will add preparers when executing the test method
test_class_instance.recording_processors.append(self) |
causing these newly added preparers to be called after ContentLengthProcessor
# fails with azure.core.exceptions.IncompleteReadError: | ||
# https://github.com/Azure/azure-sdk-for-python/pull/20888 | ||
if is_text_payload(response) and response['body']['string'] and 'content-length' in response['headers']: | ||
response['headers']['content-length'][0] = str(len(response['body']['string'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why response['headers']['content-length'] is list while request.headers['Content-Length'] is str ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good question. Because request
is a vcr.request.Request
object while response
is a dict
. vcrpy
defines different data structures for them, though I don't know why.
Description
When the test framework replaces a random name, its length is not preserved:
This make
Content-Length
header in the response not match the actual body, causing failure due to the newazure-core
check Azure/azure-sdk-for-python#20888.There are several processors that changes the
Content-Length
of the body:AbstractPreparer
orSingleValueReplacer
, likeResourceGroupPreparer
AADGraphUserReplacer
GraphClientPasswordReplacer
This makes it difficult to enforce all processors to rectify the
Content-Length
header. The best place is to rectifyContent-Length
header when all processors are finished.See
Content-Length
header's value in recordings #20541Content-Length
match the body #20533