From 3c1ce77d483accc3cfafb9a1127a7978e872d186 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Mon, 10 Jan 2022 15:32:06 +0000
Subject: [PATCH] Properly maintain aspect ratio of inline images (#7503)

---
 res/css/views/rooms/_EventTile.scss          |  9 ++++++++
 src/HtmlUtils.tsx                            | 24 ++++++++++++++------
 test/components/structures/GroupView-test.js |  3 ++-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/res/css/views/rooms/_EventTile.scss b/res/css/views/rooms/_EventTile.scss
index acc0066572..1de668985f 100644
--- a/res/css/views/rooms/_EventTile.scss
+++ b/res/css/views/rooms/_EventTile.scss
@@ -450,6 +450,15 @@ $left-gutter: 64px;
     pre {
         border: 1px solid transparent;
     }
+
+    // selector wrongly applies to pill avatars but those have explicit width/height passed at a higher specificity
+    &.markdown-body img {
+        // the image will have max-width and max-height applied during sanitization
+        width: 100%;
+        height: 100%;
+        object-fit: contain;
+        object-position: left top;
+    }
 }
 
 .mx_EventTile_clamp {
diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx
index f6c045714a..0b94cfd891 100644
--- a/src/HtmlUtils.tsx
+++ b/src/HtmlUtils.tsx
@@ -207,8 +207,12 @@ const transformTags: IExtendedSanitizeOptions["transformTags"] = { // custom to
             return { tagName, attribs: {} };
         }
 
-        const width = Number(attribs.width) || 800;
-        const height = Number(attribs.height) || 600;
+        const width = Math.min(Number(attribs.width) || 800, 800);
+        const height = Math.min(Number(attribs.height) || 600, 600);
+        // specify width/height as max values instead of absolute ones to allow object-fit to do its thing
+        // we only allow our own styles for this tag so overwrite the attribute
+        attribs.style = `max-width: ${width}px; max-height: ${height}px;`;
+
         attribs.src = mediaFromMxc(src).getThumbnailOfSourceHttp(width, height);
         return { tagName, attribs };
     },
@@ -223,9 +227,12 @@ const transformTags: IExtendedSanitizeOptions["transformTags"] = { // custom to
         return { tagName, attribs };
     },
     '*': function(tagName: string, attribs: sanitizeHtml.Attributes) {
-        // Delete any style previously assigned, style is an allowedTag for font and span
-        // because attributes are stripped after transforming
-        delete attribs.style;
+        // Delete any style previously assigned, style is an allowedTag for font, span & img,
+        // because attributes are stripped after transforming.
+        // For img this is trusted as it is generated wholly within the img transformation method.
+        if (tagName !== "img") {
+            delete attribs.style;
+        }
 
         // Sanitise and transform data-mx-color and data-mx-bg-color to their CSS
         // equivalents
@@ -249,7 +256,7 @@ const transformTags: IExtendedSanitizeOptions["transformTags"] = { // custom to
         });
 
         if (style) {
-            attribs.style = style;
+            attribs.style = style + (attribs.style || "");
         }
 
         return { tagName, attribs };
@@ -266,12 +273,15 @@ const sanitizeHtmlParams: IExtendedSanitizeOptions = {
         'details', 'summary',
     ],
     allowedAttributes: {
+        // attribute sanitization happens after transformations, so we have to accept `style` for font, span & img
+        // but strip during the transformation.
         // custom ones first:
         font: ['color', 'data-mx-bg-color', 'data-mx-color', 'style'], // custom to matrix
         span: ['data-mx-maths', 'data-mx-bg-color', 'data-mx-color', 'data-mx-spoiler', 'style'], // custom to matrix
         div: ['data-mx-maths'],
         a: ['href', 'name', 'target', 'rel'], // remote target: custom to matrix
-        img: ['src', 'width', 'height', 'alt', 'title'],
+        // img tags also accept width/height, we just map those to max-width & max-height during transformation
+        img: ['src', 'alt', 'title', 'style'],
         ol: ['start'],
         code: ['class'], // We don't actually allow all classes, we filter them in transformTags
     },
diff --git a/test/components/structures/GroupView-test.js b/test/components/structures/GroupView-test.js
index 2fbea5ce7c..9d46b2fe71 100644
--- a/test/components/structures/GroupView-test.js
+++ b/test/components/structures/GroupView-test.js
@@ -264,7 +264,8 @@ describe('GroupView', function() {
 
             const imgSrc = "https://my.home.server/_matrix/media/r0/thumbnail/someimageurl" +
                 "?width=800&amp;height=600&amp;method=scale";
-            expect(longDescElement.innerHTML).toContain('<img src="' + imgSrc + '">');
+            expect(longDescElement.innerHTML).toContain('<img src="' + imgSrc + '" ' +
+                'style="max-width:800px;max-height:600px">');
         });
 
         httpBackend