-
Notifications
You must be signed in to change notification settings - Fork 63
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
Support extension of JSON mimetype #167
Conversation
msrest/pipeline/universal.py
Outdated
'text/json' # Because we're open minded people... | ||
] | ||
# Accept "text" because we're open minded people... | ||
JSON_REGEXP = re.compile(r'(application|text)/([a-z+.]+\+)?json') |
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.
This still won't match content types with parameters:
Example: Content-type: application/json; charset=utf-8
Not blocking the PR for this. Just letting you know...
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.
Unless the parameters have already been stripped off (which it looks like they may have been at this point)
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.
Yes, they have at this point (see a little later in the file).
raw_deserializer.on_response(None, response, stream=False) | ||
result = response.context["deserialized_data"] | ||
assert result["success"] is True | ||
|
||
# JSON with UTF-8 BOM | ||
response = build_response(b'\xef\xbb\xbf{"success": true}', content_type="application/json; charset=utf-8") |
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.
Isn't adding a BOM in violation of https://tools.ietf.org/html/rfc7158#section-8.1?
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.
Codecov Report
@@ Coverage Diff @@
## master #167 +/- ##
==========================================
+ Coverage 87.84% 87.84% +<.01%
==========================================
Files 25 25
Lines 2583 2584 +1
==========================================
+ Hits 2269 2270 +1
Misses 314 314
Continue to review full report at Codecov.
|
Fix #140