Skip to content
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

Be more strict when checking if mimetype is allowed inline #154

Merged
merged 3 commits into from
Oct 28, 2023

Conversation

mauritsvanrees
Copy link
Member

This takes over some code from zopefoundation/Zope#1167

@mister-roboto
Copy link

@mauritsvanrees thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@mauritsvanrees
Copy link
Member Author

@jenkins-plone-org please run jobs

Copy link
Member

@gforcada gforcada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍🏾

Should we copy a test also from zope#1167 ?

    def test_extract_media_type(self):
        extract = OFS.Image.extract_media_type
        self.assertIsNone(extract(None))
        self.assertEqual(extract("text/plain"), "text/plain")
        self.assertEqual(extract("TEXT/PLAIN"), "text/plain")
        self.assertEqual(extract("text / plain"), "text/plain")
        self.assertEqual(extract(" text/plain ; charset=utf-8"), "text/plain")

And maintenance-wise, should we actually try to import first that function from Zope, and only if not there, use our own copy? So we can eventually drop our copied version? 🤔

@mauritsvanrees
Copy link
Member Author

@gforcada Conditional import added, and test added.

@jenkins-plone-org please run jobs

@davisagli davisagli merged commit 17a21d4 into master Oct 28, 2023
12 checks passed
@davisagli davisagli deleted the maurits-extract-media-type-master branch October 28, 2023 17:00
tisto pushed a commit to plone/plone.restapi that referenced this pull request Oct 28, 2023
* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/main
Date: 2023-10-28T22:15:39+02:00
Author: Maurits van Rees (mauritsvanrees) <[email protected]>
Commit: plone/plone.restapi@f37cd9c

Be more strict when checking if mimetype is allowed inline. (#1724)

* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.

Files changed:
A news/1167.bugfix
M src/plone/restapi/services/users/get.py
M src/plone/restapi/tests/test_services_users.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Oct 28, 2023
Branch: refs/heads/main
Date: 2023-10-28T22:15:39+02:00
Author: Maurits van Rees (mauritsvanrees) <[email protected]>
Commit: plone/plone.restapi@f37cd9c

Be more strict when checking if mimetype is allowed inline. (#1724)

* Be more strict when checking if mimetype is allowed to be displayed inline.

This takes over some code from zopefoundation/Zope#1167.
See also plone/plone.namedfile#154

* Copy a test for extract_media_type from Zope.

* Rename extract_media_type to _extract_media_type to signal it as private.

Files changed:
A news/1167.bugfix
M src/plone/restapi/services/users/get.py
M src/plone/restapi/tests/test_services_users.py
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