-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Make image plugin to attempt to detect SVG types #1440
Conversation
If known image detection fails, and if the beginning of the data is either XML or SVG header, then set mimetype to image/svg+xml.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
@@ -314,6 +314,10 @@ def _serve_individual_image(self, request): | |||
data = self._get_individual_image(run, tag, index, sample) | |||
image_type = imghdr.what(None, data) | |||
content_type = _IMGHDR_TO_MIMETYPE.get(image_type, _DEFAULT_IMAGE_MIMETYPE) | |||
if content_type == _DEFAULT_IMAGE_MIMETYPE: |
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.
Thanks for the change to add SVG support!
Do you mind adapting this logic to integrate with imghdr
? The module has a imghdr.tests
variable that you can extend to add support for new image types. That way SVG doesn't have to be special-cased as a fallback here. See https://github.com/python/cpython/blob/3.6/Lib/imghdr.py#L32
I believe the code would look like
import imghdr
def detect_svg(data, f):
if data.startswith(b'<?xml ') or data.startswith(b'<svg '):
return 'svg'
imghdr.tests.append(detect_svg)
Then you'd just add the appropriate entry to _IMGHDR_TO_MIMETYPE and the code here should work without any changes.
Thanks! I'll try to see if making changes to imghdr is acceptable. |
CLAs look good, thanks! |
@ssjhv Just to be clear, I didn't mean actually changing the imghdr module itself. The module provides a mutable list of tests as the "extension API" where you "register" a new test by appending it to that mutable module-level list. So the code I'd sketched out would just be added to the top of the images plugin file. |
Extended imghdr.tests to include svg detection. Removed previous special handling of svg.
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.
LGTM, thanks for the update.
If known image detection fails, and if the beginning of the data is either XML or SVG header, then set mimetype to image/svg+xml.