Page MenuHomeSoftware Heritage
Differential D6107

browse/utils: Fix edge case in prepare_content_for_display
ClosedPublic
Actions

Authored by anlambert on Aug 17 2021, 3:30 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T3486: Content with mime type application/pgp-keys and encoding us-ascii cannot be displayed.
Commits
rDWAPPSc9d89d93b7df: browse/utils: Fix edge case in prepare_content_for_display
Summary

Contents with mimetype starting with application/ but with
no detected programming language were not processed for
display but they can be regular text files.

Closes T3486

Test Plan

This was really an edge case, nevertheless I have updated tests
to cover it using the same file as in the task.

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Aug 17 2021, 3:30 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 17 2021, 3:30 PM
swh-public-ci added a comment.Aug 17 2021, 3:49 PM

Build has FAILED

Patch application report for D6107 (id=22092)

Rebasing onto ddfb988db5...

Current branch diff-target is up to date.
Changes applied before test
commit 1ea3704d673e7945ad12d70090872db06db08791
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Aug 17 15:24:44 2021 +0200

    browse/utils: Fix edge case in prepare_content_for_display
    
    Contents with mimetype starting with application/ but with
    no detected programming language were not processed for
    display but they can be regular text files.
    
    Closes T3486

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1002/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1002/console

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 17 2021, 3:49 PM
Harbormaster failed remote builds in B23049: Diff 22092!
anlambert updated this revision to Diff 22094.Aug 17 2021, 4:58 PM

Fix cypress test

swh-public-ci added a comment.Aug 17 2021, 5:11 PM

Build is green

Patch application report for D6107 (id=22094)

Rebasing onto ddfb988db5...

Current branch diff-target is up to date.
Changes applied before test
commit c9d89d93b7dfb8812f02f7d2bbafa27aed152035
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Aug 17 15:24:44 2021 +0200

    browse/utils: Fix edge case in prepare_content_for_display
    
    Contents with mimetype starting with application/ but with
    no detected programming language were not processed for
    display but they can be regular text files.
    
    Closes T3486

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1004/ for more details.

Harbormaster completed remote builds in B23051: Diff 22094.Aug 17 2021, 5:11 PM
anlambert requested review of this revision.Aug 17 2021, 5:11 PM
vlorentz accepted this revision.Aug 18 2021, 11:12 AM
vlorentz added a subscriber: vlorentz.

thanks

This revision is now accepted and ready to land.Aug 18 2021, 11:12 AM
Closed by commit rDWAPPSc9d89d93b7df: browse/utils: Fix edge case in prepare_content_for_display (authored by anlambert). · Explain WhyAug 18 2021, 12:05 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPSc9d89d93b7df: browse/utils: Fix edge case in prepare_content_for_display.

Revision Contents
Changeset List

PathSize
swh/
web/
browse/
6 lines
templates/
includes/
2 lines
tests/
browse/
views/
39 lines
14 lines
resources/
contents/
other/
extensions/
360 lines
17 lines

Diff 22112

View Options

swh/web/browse/utils.py

# Copyright (C) 2017-2020 The Software Heritage developers # Copyright (C) 2017-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version # License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
import base64 import base64
import stat import stat
import textwrap import textwrap
from threading import Lock from threading import Lock
▲ Show 20 Lines • Show All 219 Lines • ▼ Show 20 Linesdef prepare_content_for_display(content_data, mime_type, path):
language = highlightjs.get_hljs_language_from_filename(path) language = highlightjs.get_hljs_language_from_filename(path)
if not language: if not language:
language = highlightjs.get_hljs_language_from_mime_type(mime_type) language = highlightjs.get_hljs_language_from_mime_type(mime_type)
if not language: if not language:
language = "nohighlight" language = "nohighlight"
elif mime_type.startswith("application/"):
mime_type = mime_type.replace("application/", "text/")
if mime_type.startswith("image/"): if mime_type.startswith("image/"):
if mime_type in browsers_supported_image_mimes: if mime_type in browsers_supported_image_mimes:
content_data = base64.b64encode(content_data).decode("ascii") content_data = base64.b64encode(content_data).decode("ascii")
if mime_type.startswith("image/svg"): if mime_type.startswith("image/svg"):
mime_type = "image/svg+xml" mime_type = "image/svg+xml"
if mime_type.startswith("text/"): if mime_type.startswith("text/") or mime_type.startswith("application/"):
content_data = content_data.decode("utf-8", errors="replace") content_data = content_data.decode("utf-8", errors="replace")
return {"content_data": content_data, "language": language, "mimetype": mime_type} return {"content_data": content_data, "language": language, "mimetype": mime_type}
def gen_link(url, link_text=None, link_attrs=None): def gen_link(url, link_text=None, link_attrs=None):
""" """
Utility function for generating an HTML link to insert Utility function for generating an HTML link to insert
▲ Show 20 Lines • Show All 468 Lines • Show Last 20 Lines
View Options

swh/web/templates/includes/content-display.html

Show All 20 Lines <div class="card">
<div class="swh-content"> <div class="swh-content">
{% if content_size > max_content_size %} {% if content_size > max_content_size %}
Content is too large to be displayed (size is greater than {{ max_content_size|filesizeformat }}). Content is too large to be displayed (size is greater than {{ max_content_size|filesizeformat }}).
{% elif "inode/x-empty" == mimetype %} {% elif "inode/x-empty" == mimetype %}
<i>File is empty</i> <i>File is empty</i>
{% elif filename and filename|default:""|slice:"-5:" == "ipynb" %} {% elif filename and filename|default:""|slice:"-5:" == "ipynb" %}
<div class="swh-ipynb"> <div class="swh-ipynb">
</div> </div>
{% elif "text/" in mimetype and encoding != "binary" %} {% elif "text/" in mimetype or "application/" in mimetype and encoding != "binary" %}
<div class="highlightjs"> <div class="highlightjs">
<pre><code class="{{ language }}">{{ content }}</code></pre> <pre><code class="{{ language }}">{{ content }}</code></pre>
</div> </div>
{% elif mimetype in browsers_supported_image_mimes and content %} {% elif mimetype in browsers_supported_image_mimes and content %}
<img src="data:{{ mimetype }};base64,{{ content }}"/> <img src="data:{{ mimetype }};base64,{{ content }}"/>
{% elif "application/pdf" == mimetype %} {% elif "application/pdf" == mimetype %}
<div class="text-center"> <div class="text-center">
<div class="py-2"> <div class="py-2">
▲ Show 20 Lines • Show All 42 Lines • Show Last 20 Lines
View Options

swh/web/tests/browse/views/test_content.py

# Copyright (C) 2017-2020 The Software Heritage developers # Copyright (C) 2017-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version # License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
import random import random
from hypothesis import given from hypothesis import given
from django.utils.html import escape from django.utils.html import escape
from swh.model.identifiers import CONTENT, DIRECTORY, RELEASE, REVISION, SNAPSHOT from swh.model.identifiers import CONTENT, DIRECTORY, RELEASE, REVISION, SNAPSHOT
from swh.web.browse.snapshot_context import process_snapshot_branches from swh.web.browse.snapshot_context import process_snapshot_branches
from swh.web.browse.utils import ( from swh.web.browse.utils import (
_re_encode_content, _re_encode_content,
get_mimetype_and_encoding_for_content, get_mimetype_and_encoding_for_content,
prepare_content_for_display, prepare_content_for_display,
) )
from swh.web.common.exc import NotFoundExc from swh.web.common.exc import NotFoundExc
from swh.web.common.identifiers import gen_swhid from swh.web.common.identifiers import gen_swhid
from swh.web.common.utils import gen_path_info, reverse from swh.web.common.utils import gen_path_info, reverse
from swh.web.tests.django_asserts import assert_contains, assert_not_contains from swh.web.tests.django_asserts import assert_contains, assert_not_contains
from swh.web.tests.strategies import ( from swh.web.tests.strategies import (
content, content,
content_application_no_highlight,
content_image_type, content_image_type,
content_text, content_text,
content_text_no_highlight, content_text_no_highlight,
content_text_non_utf8, content_text_non_utf8,
content_unsupported_image_type_rendering, content_unsupported_image_type_rendering,
content_utf8_detected_as_binary, content_utf8_detected_as_binary,
invalid_sha1, invalid_sha1,
origin_with_multiple_visits, origin_with_multiple_visits,
Show All 28 Linesdef test_content_view_text(client, archive_data, content):
swh_cnt_id = gen_swhid(CONTENT, sha1_git) swh_cnt_id = gen_swhid(CONTENT, sha1_git)
swh_cnt_id_url = reverse("browse-swhid", url_args={"swhid": swh_cnt_id}) swh_cnt_id_url = reverse("browse-swhid", url_args={"swhid": swh_cnt_id})
assert_contains(resp, swh_cnt_id) assert_contains(resp, swh_cnt_id)
assert_contains(resp, swh_cnt_id_url) assert_contains(resp, swh_cnt_id_url)
assert_not_contains(resp, "swh-metadata-popover") assert_not_contains(resp, "swh-metadata-popover")
@given(content_text_no_highlight()) @given(content_application_no_highlight(), content_text_no_highlight())
def test_content_view_text_no_highlight(client, archive_data, content): def test_content_view_no_highlight(client, archive_data, content_app, content_text):
for content_ in (content_app, content_text):
content = content_
sha1_git = content["sha1_git"] sha1_git = content["sha1_git"]
url = reverse("browse-content", url_args={"query_string": content["sha1"]}) url = reverse("browse-content", url_args={"query_string": content["sha1"]})
url_raw = reverse("browse-content-raw", url_args={"query_string": content["sha1"]}) url_raw = reverse(
"browse-content-raw", url_args={"query_string": content["sha1"]}
)
resp = check_html_get_response( resp = check_html_get_response(
client, url, status_code=200, template_used="browse/content.html" client, url, status_code=200, template_used="browse/content.html"
) )
content_display = _process_content_for_display(archive_data, content) content_display = _process_content_for_display(archive_data, content)
assert_contains(resp, '<code class="nohighlight">') assert_contains(resp, '<code class="nohighlight">')
assert_contains(resp, escape(content_display["content_data"])) assert_contains(resp, escape(content_display["content_data"]))
assert_contains(resp, url_raw) assert_contains(resp, url_raw)
swh_cnt_id = gen_swhid(CONTENT, sha1_git) swh_cnt_id = gen_swhid(CONTENT, sha1_git)
swh_cnt_id_url = reverse("browse-swhid", url_args={"swhid": swh_cnt_id}) swh_cnt_id_url = reverse("browse-swhid", url_args={"swhid": swh_cnt_id})
assert_contains(resp, swh_cnt_id) assert_contains(resp, swh_cnt_id)
assert_contains(resp, swh_cnt_id_url) assert_contains(resp, swh_cnt_id_url)
@given(content_text_non_utf8()) @given(content_text_non_utf8())
def test_content_view_no_utf8_text(client, archive_data, content): def test_content_view_no_utf8_text(client, archive_data, content):
sha1_git = content["sha1_git"] sha1_git = content["sha1_git"]
url = reverse("browse-content", url_args={"query_string": content["sha1"]}) url = reverse("browse-content", url_args={"query_string": content["sha1"]})
▲ Show 20 Lines • Show All 513 Lines • Show Last 20 Lines
View Options

swh/web/tests/data.py

# Copyright (C) 2018-2020 The Software Heritage developers # Copyright (C) 2018-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version # License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from copy import deepcopy from copy import deepcopy
from datetime import timedelta from datetime import timedelta
import os import os
from pathlib import Path
import random import random
import time import time
from typing import Dict, List, Optional, Set from typing import Dict, List, Optional, Set
from swh.core.config import merge_configs from swh.core.config import merge_configs
from swh.counters import get_counters from swh.counters import get_counters
from swh.indexer.ctags import CtagsIndexer from swh.indexer.ctags import CtagsIndexer
from swh.indexer.fossology_license import FossologyLicenseIndexer from swh.indexer.fossology_license import FossologyLicenseIndexer
▲ Show 20 Lines • Show All 149 Lines • ▼ Show 20 Lines
0 1 0 0 0 0 0 1 1 1 1 0 0 1 1 1 1 0 0 1 0 0 0 0 0 1 0 0 0 0 0 1 1 1 1 0 0 1 1 1 1 0 0 1 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0""" 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0"""
# add file with mimetype image/x-portable-bitmap in the archive content # add file with mimetype image/x-portable-bitmap in the archive content
pbm_content = Content.from_data(pbm_image_data) pbm_content = Content.from_data(pbm_image_data)
storage.content_add([pbm_content]) storage.content_add([pbm_content])
contents.add(pbm_content.sha1) contents.add(pbm_content.sha1)
# add file with mimetype application/pgp-keys in the archive content
gpg_path = os.path.join(
os.path.dirname(__file__), "resources/contents/other/extensions/public.gpg"
)
gpg_content = Content.from_data(Path(gpg_path).read_bytes())
storage.content_add([gpg_content])
contents.add(gpg_content.sha1)
INDEXER_TOOL = { INDEXER_TOOL = {
"tool_name": "swh-web tests", "tool_name": "swh-web tests",
"tool_version": "1.0", "tool_version": "1.0",
"tool_configuration": {}, "tool_configuration": {},
} }
ORIGIN_METADATA_KEY = "keywords" ORIGIN_METADATA_KEY = "keywords"
▲ Show 20 Lines • Show All 156 Lines • ▼ Show 20 Lines for content in result:
path = "" path = ""
if content.sha1 in content_path: if content.sha1 in content_path:
path = content_path[content.sha1] path = content_path[content.sha1]
cnt_data = storage.content_get_data(content.sha1) cnt_data = storage.content_get_data(content.sha1)
assert cnt_data is not None assert cnt_data is not None
mimetype, encoding = get_mimetype_and_encoding_for_content(cnt_data) mimetype, encoding = get_mimetype_and_encoding_for_content(cnt_data)
_, _, cnt_data = _re_encode_content(mimetype, encoding, cnt_data) _, _, cnt_data = _re_encode_content(mimetype, encoding, cnt_data)
content_display_data = prepare_content_for_display(cnt_data, mimetype, path) content_display_data = prepare_content_for_display(cnt_data, mimetype, path)
content_metadata.update( content_metadata.update(
{ {
"path": path, "path": path,
"mimetype": mimetype, "mimetype": mimetype,
"encoding": encoding, "encoding": encoding,
"hljs_language": content_display_data["language"], "hljs_language": content_display_data["language"],
"raw_data": cnt_data,
"data": content_display_data["content_data"], "data": content_display_data["content_data"],
} }
) )
_contents[sha1] = content_metadata _contents[sha1] = content_metadata
contents.append(content_metadata) contents.append(content_metadata)
# Add the empty directory to the test archive # Add the empty directory to the test archive
storage.directory_add([Directory(entries=())]) storage.directory_add([Directory(entries=())])
# Add empty content to the test archive # Add empty content to the test archive
storage.content_add([Content.from_data(data=b"")]) storage.content_add([Content.from_data(data=b"")])
▲ Show 20 Lines • Show All 112 Lines • Show Last 20 Lines
View Options

swh/web/tests/resources/contents/other/extensions/public.gpg

Loading...
View Options

swh/web/tests/strategies.py

Loading...
About · Developer information · Legal