From b66bcc3d1b2cc41f50f748c807f17e0fe3e10523 Mon Sep 17 00:00:00 2001 From: Ross Masters Date: Sat, 14 Dec 2024 12:04:41 +0000 Subject: [PATCH 1/4] Test out removing the og:image if the UA is the Slack link expander --- warehouse/packaging/views.py | 4 ++++ warehouse/templates/base.html | 2 ++ 2 files changed, 6 insertions(+) diff --git a/warehouse/packaging/views.py b/warehouse/packaging/views.py index 55f722c43c0b..6a20c1e34398 100644 --- a/warehouse/packaging/views.py +++ b/warehouse/packaging/views.py @@ -275,6 +275,9 @@ def release_detail(release, request): key=lambda f: f.filename, ) + # TODO: Move somewhere more sensible, ignore version + is_slackbot = request.user_agent == "Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)" + return { "project": project, "release": release, @@ -288,6 +291,7 @@ def release_detail(release, request): "license": license, # Additional function to format the attestations "PEP740AttestationViewer": PEP740AttestationViewer, + "is_slackbot": is_slackbot, } diff --git a/warehouse/templates/base.html b/warehouse/templates/base.html index bdd7f2054c5e..732d11d7727a 100644 --- a/warehouse/templates/base.html +++ b/warehouse/templates/base.html @@ -113,7 +113,9 @@ + {% if not is_slackbot %} + {% endif %} {% block extra_meta %}{% endblock %} From ec310873e95afeb8175831589d2afd2608e7b2e9 Mon Sep 17 00:00:00 2001 From: Ross Masters Date: Sat, 14 Dec 2024 12:32:02 +0000 Subject: [PATCH 2/4] Extract to function, add tests, add pypi twitter handle --- tests/unit/utils/test_user_agents.py | 39 ++++++++++++++++++++++++++++ warehouse/packaging/views.py | 6 ++--- warehouse/templates/base.html | 4 ++- warehouse/utils/user_agents.py | 21 +++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 tests/unit/utils/test_user_agents.py create mode 100644 warehouse/utils/user_agents.py diff --git a/tests/unit/utils/test_user_agents.py b/tests/unit/utils/test_user_agents.py new file mode 100644 index 000000000000..3420f8402ed3 --- /dev/null +++ b/tests/unit/utils/test_user_agents.py @@ -0,0 +1,39 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from warehouse.utils.user_agents import should_show_share_image + + +def test_shows_share_image_for_social_networks() -> None: + # https://developer.x.com/en/docs/x-for-websites/cards/guides/troubleshooting-cards#validate_twitterbot + assert should_show_share_image("Twitterbot/1.0") is True + # https://developers.facebook.com/docs/sharing/webmasters/web-crawlers + assert ( + should_show_share_image( + "facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)" + ) + is True + ) + assert should_show_share_image("facebookexternalhit/1.1") is True + assert should_show_share_image("facebookcatalog/1.0") is True + # https://www.linkedin.com/robots.txt + assert should_show_share_image("LinkedInBot") is True + + +def test_doesnt_show_share_image_for_slackbot() -> None: + # https://api.slack.com/robots + assert ( + should_show_share_image( + "Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)" + ) + is False + ) diff --git a/warehouse/packaging/views.py b/warehouse/packaging/views.py index 6a20c1e34398..3b78c89bb403 100644 --- a/warehouse/packaging/views.py +++ b/warehouse/packaging/views.py @@ -30,6 +30,7 @@ from warehouse.observations.models import ObservationKind from warehouse.packaging.forms import SubmitMalwareObservationForm from warehouse.packaging.models import Description, File, Project, Release, Role +from warehouse.utils.user_agents import should_show_share_image class PEP740AttestationViewer: @@ -275,9 +276,6 @@ def release_detail(release, request): key=lambda f: f.filename, ) - # TODO: Move somewhere more sensible, ignore version - is_slackbot = request.user_agent == "Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)" - return { "project": project, "release": release, @@ -291,7 +289,7 @@ def release_detail(release, request): "license": license, # Additional function to format the attestations "PEP740AttestationViewer": PEP740AttestationViewer, - "is_slackbot": is_slackbot, + "show_share_image": should_show_share_image(request), } diff --git a/warehouse/templates/base.html b/warehouse/templates/base.html index 732d11d7727a..44fcd324f626 100644 --- a/warehouse/templates/base.html +++ b/warehouse/templates/base.html @@ -113,11 +113,13 @@ - {% if not is_slackbot %} + {% if show_share_image %} {% endif %} + + {% block extra_meta %}{% endblock %} diff --git a/warehouse/utils/user_agents.py b/warehouse/utils/user_agents.py new file mode 100644 index 000000000000..30175490dabd --- /dev/null +++ b/warehouse/utils/user_agents.py @@ -0,0 +1,21 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from pyramid.request import Request + + +def should_show_share_image(request: Request) -> bool: + if user_agent := request.user_agent: + if user_agent.strip().startswith("Slackbot-LinkExpanding"): + return False + + return True From 52bf053e9e180f9b814f56c931f78535097ee80a Mon Sep 17 00:00:00 2001 From: Ross Masters Date: Thu, 19 Dec 2024 20:37:51 +0000 Subject: [PATCH 3/4] fix: Handle some edge-cases for user-agents webob.Request.user_agent should be a string or None, as per: https://github.com/Pylons/webob/blob/39d5af3c797e7b867f152c2e8c979de42d029403/src/webob/request.py#L1185 --- tests/unit/utils/test_user_agents.py | 7 +++++++ warehouse/utils/user_agents.py | 12 ++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/unit/utils/test_user_agents.py b/tests/unit/utils/test_user_agents.py index 3420f8402ed3..78e58d613367 100644 --- a/tests/unit/utils/test_user_agents.py +++ b/tests/unit/utils/test_user_agents.py @@ -13,6 +13,13 @@ from warehouse.utils.user_agents import should_show_share_image +def test_default_show_image() -> None: + # Missing user-agent header + assert should_show_share_image(None) is True + # Empty user-agent header + assert should_show_share_image("") is True + + def test_shows_share_image_for_social_networks() -> None: # https://developer.x.com/en/docs/x-for-websites/cards/guides/troubleshooting-cards#validate_twitterbot assert should_show_share_image("Twitterbot/1.0") is True diff --git a/warehouse/utils/user_agents.py b/warehouse/utils/user_agents.py index 30175490dabd..42ab40309970 100644 --- a/warehouse/utils/user_agents.py +++ b/warehouse/utils/user_agents.py @@ -13,9 +13,13 @@ from pyramid.request import Request -def should_show_share_image(request: Request) -> bool: - if user_agent := request.user_agent: - if user_agent.strip().startswith("Slackbot-LinkExpanding"): - return False +def should_show_share_image(user_agent: str | None) -> bool: + # User agent header not included or empty + if not user_agent: + return True + + # Don't show the og:image for Slackbot link-expansion requests + if user_agent.strip().startswith("Slackbot-LinkExpanding"): + return False return True From e2333a2c6d06f3a0bd1f90598658f2ea7b24d727 Mon Sep 17 00:00:00 2001 From: Ross Masters Date: Thu, 19 Dec 2024 20:50:44 +0000 Subject: [PATCH 4/4] Refactored to use a jinja filter --- tests/unit/test_filters.py | 33 ++++++++++++++++++++ tests/unit/utils/test_user_agents.py | 46 ---------------------------- warehouse/config.py | 1 + warehouse/filters.py | 19 ++++++++++++ warehouse/packaging/views.py | 2 -- warehouse/templates/base.html | 2 +- warehouse/utils/user_agents.py | 25 --------------- 7 files changed, 54 insertions(+), 74 deletions(-) delete mode 100644 tests/unit/utils/test_user_agents.py delete mode 100644 warehouse/utils/user_agents.py diff --git a/tests/unit/test_filters.py b/tests/unit/test_filters.py index b7c7a2ce04b9..e78ca40918f0 100644 --- a/tests/unit/test_filters.py +++ b/tests/unit/test_filters.py @@ -301,3 +301,36 @@ def test_remove_invalid_xml_unicode(inp, expected): Test that invalid XML unicode characters are removed. """ assert filters.remove_invalid_xml_unicode(inp) == expected + + +def test_show_share_image(): + # Missing user-agent header + assert filters.show_share_image(None) is True + # Empty user-agent header + assert filters.show_share_image("") is True + + # Twitter/X - shows image + # https://developer.x.com/en/docs/x-for-websites/cards/guides/troubleshooting-cards#validate_twitterbot + assert filters.show_share_image("Twitterbot/1.0") is True + + # Facebook - shows image + # https://developers.facebook.com/docs/sharing/webmasters/web-crawlers + assert ( + filters.show_share_image( + "facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)" + ) + is True + ) + assert filters.show_share_image("facebookexternalhit/1.1") is True + assert filters.show_share_image("facebookcatalog/1.0") is True + + # LinkedIn - shows image (https://www.linkedin.com/robots.txt) + assert filters.show_share_image("LinkedInBot") is True + + # Slack - don't show image (https://api.slack.com/robots) + assert ( + filters.show_share_image( + "Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)" + ) + is False + ) diff --git a/tests/unit/utils/test_user_agents.py b/tests/unit/utils/test_user_agents.py deleted file mode 100644 index 78e58d613367..000000000000 --- a/tests/unit/utils/test_user_agents.py +++ /dev/null @@ -1,46 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from warehouse.utils.user_agents import should_show_share_image - - -def test_default_show_image() -> None: - # Missing user-agent header - assert should_show_share_image(None) is True - # Empty user-agent header - assert should_show_share_image("") is True - - -def test_shows_share_image_for_social_networks() -> None: - # https://developer.x.com/en/docs/x-for-websites/cards/guides/troubleshooting-cards#validate_twitterbot - assert should_show_share_image("Twitterbot/1.0") is True - # https://developers.facebook.com/docs/sharing/webmasters/web-crawlers - assert ( - should_show_share_image( - "facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)" - ) - is True - ) - assert should_show_share_image("facebookexternalhit/1.1") is True - assert should_show_share_image("facebookcatalog/1.0") is True - # https://www.linkedin.com/robots.txt - assert should_show_share_image("LinkedInBot") is True - - -def test_doesnt_show_share_image_for_slackbot() -> None: - # https://api.slack.com/robots - assert ( - should_show_share_image( - "Slackbot-LinkExpanding 1.0 (+https://api.slack.com/robots)" - ) - is False - ) diff --git a/warehouse/config.py b/warehouse/config.py index 1014a1ca6dbe..2e025e2b2350 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -683,6 +683,7 @@ def configure(settings=None): filters.setdefault( "remove_invalid_xml_unicode", "warehouse.filters:remove_invalid_xml_unicode" ) + filters.setdefault("show_share_image", "warehouse.filters:show_share_image") # We also want to register some global functions for Jinja jglobals = config.get_settings().setdefault("jinja2.globals", {}) diff --git a/warehouse/filters.py b/warehouse/filters.py index 68364adfc6d3..43f2125b519e 100644 --- a/warehouse/filters.py +++ b/warehouse/filters.py @@ -201,3 +201,22 @@ def remove_invalid_xml_unicode(value: str | None) -> str | None: def includeme(config): config.add_request_method(_camo_url, name="camo_url") + + +def show_share_image(user_agent: str | None) -> bool: + """ + Whether the og:image meta-tag should be included based on the user-agent + + Used to exclude the image from Slack link-expansion. + + """ + + # User agent header not included or empty + if not user_agent: + return True + + # Don't show the og:image for Slackbot link-expansion requests + if user_agent.strip().startswith("Slackbot-LinkExpanding"): + return False + + return True diff --git a/warehouse/packaging/views.py b/warehouse/packaging/views.py index 3b78c89bb403..55f722c43c0b 100644 --- a/warehouse/packaging/views.py +++ b/warehouse/packaging/views.py @@ -30,7 +30,6 @@ from warehouse.observations.models import ObservationKind from warehouse.packaging.forms import SubmitMalwareObservationForm from warehouse.packaging.models import Description, File, Project, Release, Role -from warehouse.utils.user_agents import should_show_share_image class PEP740AttestationViewer: @@ -289,7 +288,6 @@ def release_detail(release, request): "license": license, # Additional function to format the attestations "PEP740AttestationViewer": PEP740AttestationViewer, - "show_share_image": should_show_share_image(request), } diff --git a/warehouse/templates/base.html b/warehouse/templates/base.html index 44fcd324f626..11b371cc18c8 100644 --- a/warehouse/templates/base.html +++ b/warehouse/templates/base.html @@ -113,7 +113,7 @@ - {% if show_share_image %} + {% if request.user_agent | show_share_image %} {% endif %} diff --git a/warehouse/utils/user_agents.py b/warehouse/utils/user_agents.py deleted file mode 100644 index 42ab40309970..000000000000 --- a/warehouse/utils/user_agents.py +++ /dev/null @@ -1,25 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from pyramid.request import Request - - -def should_show_share_image(user_agent: str | None) -> bool: - # User agent header not included or empty - if not user_agent: - return True - - # Don't show the og:image for Slackbot link-expansion requests - if user_agent.strip().startswith("Slackbot-LinkExpanding"): - return False - - return True