From e4a8304f57f99f8bd32c2f63c94b2242acd7558c Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Wed, 24 Jul 2024 15:01:20 +0000 Subject: [PATCH] thumbnail: Store the post-orientation-transformation dimensions. Modern browsers respect the EXIF orientation information of images, applying rotation and/or mirroring as specified in those tags. The the `width="..."` and `height="..."` tags are to size the image _after_ applying those orientation transformations. The `.width` and `.height` properties of libvips' images are _before_ any transformations are applied. Since we intend to use these to hint to rendering clients the size that the image should be _rendered at_, change to storing (and providing to clients) the dimensions of the rendered image, not the stored bytes. --- api_docs/message-formatting.md | 3 +- zerver/lib/thumbnail.py | 16 +++++++-- zerver/tests/images/orientation.jpg | Bin 0 -> 5512 bytes zerver/tests/test_thumbnail.py | 50 ++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 zerver/tests/images/orientation.jpg diff --git a/api_docs/message-formatting.md b/api_docs/message-formatting.md index abdab6fb72..4bdd815110 100644 --- a/api_docs/message-formatting.md +++ b/api_docs/message-formatting.md @@ -86,7 +86,8 @@ previews: available. Clients that would like to size the lightbox based on the size of the original image can use the `data-original-dimensions` attribute, which encodes the dimensions of the original image as - `{width}x{height}`, to do so. + `{width}x{height}`, to do so. These dimensions are for the image as + rendered, _after_ any EXIF rotation and mirroring has been applied. - Animated images will have a `data-animated` attribute on the `img` tag. As detailed in `server_thumbnail_formats`, both animated and still images are available for clients to use, depending on their diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py index fd18392fe0..928e2fe609 100644 --- a/zerver/lib/thumbnail.py +++ b/zerver/lib/thumbnail.py @@ -295,11 +295,23 @@ def maybe_thumbnail(attachment: AbstractAttachment, content: bytes) -> ImageAtta try: # This only attempts to read the header, not the full image content with libvips_check_image(content) as image: + # "original_width_px" and "original_height_px" here are + # _as rendered_, after applying the orientation + # information which the image may contain. + if ( + "orientation" in image.get_fields() + and image.get("orientation") >= 5 + and image.get("orientation") <= 8 + ): + (width, height) = (image.height, image.width) + else: + (width, height) = (image.width, image.height) + image_row = ImageAttachment.objects.create( realm_id=attachment.realm_id, path_id=attachment.path_id, - original_width_px=image.width, - original_height_px=image.height, + original_width_px=width, + original_height_px=height, frames=image.get_n_pages(), thumbnail_metadata=[], ) diff --git a/zerver/tests/images/orientation.jpg b/zerver/tests/images/orientation.jpg new file mode 100644 index 0000000000000000000000000000000000000000..d61096b1b3e91f189a13cd4b372e3dc38d49b8de GIT binary patch literal 5512 zcmd6qXH-if{KB+N6uBxTkGEU*8O$sNAItztGlbZdaqTz^k8Wc1Y2(o4*|f* z38(-72mlKS00bonWdMl+><$J|sgR`K$ zwIwYMHRA9uS-AgW4{%4_&Xq>{rT>2snI6lIN1b#xE9kLBp+P^Jb) zau~*Nn2kD!vM`53ms#zH%`%&Fm=Volpf((zV;Rv54tJxxFEN3MGUAAGdSW;;3FW&e z%S9zbhok%nWok4tC=LJ&iKCBa(!)^JLYc(&aJNL+1WgSo^fwOvjpLb{Q9l8&jEUXC z4i62Brzz2uY1&3chBP~7QWP^jUfnf_9vQ@D&@5x3V}n>*09cNhGYbfC&X$HAvc9&F zzP`GS273HI<=+newEFkJ$?dYmd$%7sgM3+@qrHd&Cz;GTto7`Wz;9>yU_5dLK z=$B6JH~@TU0JPrxEgm7xeF=+?k2TiROiD`92xrnYI0yPu{!fFSmj4=l+fReD-_Nt7 zSusO`5~AX1oI|C@M8zbqX>qYZbS6#xe_!JNc;atv{pN?7J2QmIX0p(&ywF<~&I(1- z&0>VdhsUsJ;jDj^;s0>jZ$5Bv`Cg+ysrU}40UCh+CKVuW2LXn{4G_O$=m_*PZ;nJS z!1?pMrCu!Gdz8`fAMM{7I0x;*ap9pfj@r`QgGNtaCvq75CpZljZ~;C*1tLHa$O0vx z4s?J4SOv_1HCPK=fCume8vz}JfoQ-6Ngx&M1nD3PTfh#m2kZ}r!kge^ zI1N4k7s2Il9o!0Efv>~&;0gE*JdXf`8xcaJ5p~26u|!;u^+*W9Mz$fDNFh>=)FT~8 zA94qoK&Fw;7#xOzp<&c9t1xyLPYfNi39}88jVZxYVOlZWm^+wB%pB%BmIo`2RmB=( z?Xf=CP;3%59b1U4#I|62u=lW2*aaL8CxlbP8Q|=2zPJcn3N9OW6xV>ej2p&1#m(dK zcoDoR-W2bG55mXe)A7alI{YR4ZTuAe6M>r`MbIJG5Y`h|gk6L}LJi>};Wps~;R}(U zC{Hvdx)2$}EyNsR1+jy8lQ>2E!bRp%Z)#WWGYaGkiDsrum8d3jCJ*LHyhJOZm_7 zkMPfvDP(Q33ptvcO|BsikY5Vm1r!9V1sDQp0w)Eo3OuDC6j_QTg-+Q`si5>wrl>fo zBGsN6LCvC`rVdf(1O)_j1-%3}3ziCY3QhP& ze#Pw-ABDw)&4ro5nZgai_k1kks=30+C;`hF`_D>ZlXz|$3%NXr^TpZtHkJH znPSaikHled6>)d*6!DYdgW~fNk`lHOu@c1+S0$z;g(S@-BP8=BFG{|WqDZZl3YW^4 zx+L|QCP=fOMbV0A-LyGracMj01nCpfL(*Sl6lFYRcF8ozjLUM#8p?*q=F4`;&dEv1 zImxBU)yh4RC(0Yjhsht7@00(ips3)juvei?;iaOmqP=3WVy)ts5|5IZ(k7*HrMt>F zWkcmi<)g~CRA3c7l`xf3m7A)ts=jKt>QU8UHLRME8cVHQZA6`bG_z4%|6ZVT6$VhS`}KO+5*~ZwRdTEX#c6BqC?jy)w#QpXQj=`?JL_? zzSC9JW$GT&eV|9tbJE+Z*QNJO-$0+OU$6hlK+Yi0;E2J9A=S{;Fx#--2y3*)XuHt` zqtC|r#_`6@#S@qIH(S&JIVe-^e#x%(Egz3a;+Umg7$5&67Nt*?km76^= zmo;aYSDL@DP_~G$sI!<^qqT;;re)1XOJmEemY1y%D_g5ft08LvYftME>&G@SHla4P zHgmRmwkfuk?XY$Zb_eZ7>?Q2!_SN=tYYo##gXmU;RHLabvooU z<}B~ba&B`0F7_^mT*h4$T{pR&cf+~4x|O;;ch`1Lb?;k8Sr@RbX5E6v8jl>0QBNh$ z1kWxnJ}*D7YOjypR^EBu6F!Y{3T|`;L$4HmRQ;|zi-cb$Fgy_KNb1VT? zB&$0{GG=qkaI9)dSV;2EqGhkcG>M|+oyKe?x@Gpc{`V$R84Oxw#F`@BwQ()8mW{2jkGlpmC&&r=Y(n4v;Y=v64 zw9d7KwN149why0Md#oAzHAFdjH}&G1^=b^YtDgL;E4H}r0_4CxQG z-ZZ$`e#_`q$FRxp#oK1LuiUY^(|gzc?)7`l_lEC#-hVLSH!|^n@!;jdsE6+!B|iEx zx?>DGmO0KpUieu2arr-#|Isj^KhZg9H97cX-ILL$^rvs0#Xb8vmG+$HdEpDG7u7Fy zUS4=*^Xk^?4X>x(#J>45o&J{m_Q;IV%-LD<*}*xVxv6*TcT0a}zZZFb>Vxiwu6ft_ z#|zO5Up{7j68Tj1+3<7UqW9v9FG*kVUyHsef9v?}@O^A4dTD8C21r{lVuG1~VUE@U z5CZrS7{PJsf`7`=C`beFPeD*ZFldWluo#YDao8V%!{PA+JRV0Ra&Zxfq`wPI9xfsm4<9coe0)EQ{`!!Xdw-wzv$xa*$OPC2Fc63gU^0Y|p`~t+gH}8+f|eKm zbQZ1giC7#SgK+(>TOQ>{!$ZCJWs4dTv@k?;1^ykmkT8`DAPj`SSOf!co@{9C48dr4 zWC1*o*3y=t5cn;qJKS^xgLd9saoAAc*Fwwd=Po@0B-9z0jF3^yrOV6e|Js&!O~M2k zql1CDg-qfO0Lav;_bRNZS=3?SLu#p957gRG9C5jM?#6YJdJ)R|D(X$sOcaFCi_*(= zUJmNvE=09jeR!EklLP1*0$2&0$brt4oj~7R2ppOjY|0!!wP4esiNsqR_%9U%%^U%O z0h;#rWj`4CjUf)t)amXHqDB~N2s~TI%6s{?H{Eyj`{853{o?y}9^dAn-uG3r;Iyt= zW`gc)CnYdV8TTgh^=MgghojPXF}L-`S~zWO9l{WkGw>2lE+%pp*~$fccyq{R)5Aq= zNqkNdC8#=H+Yj*sH_yJSO*1iq5IvQ)HfS4 z1J|5RGc_o&${0?UZu7LdH^i2S$Mt;HTO}p-=)n4h&r)*18Hwf5Y#TDRMn)#tuX^-m z!?$p=@7Jy^fzkQP4@3Jkk_-BeBoE$lBOVab>AVneLoa@|PxFZnai@Q&|LYNtk+!sm zln3ycx?J=8TZ5T(aZd~OQVv@?PtzVfTLQ`-w64%>K9co^FjCc&K2Y-_az7>$ogK^}Y~MD(T+h(d{Mka({%8#QCoN_kNXE3Va?b zZcur&&|TK89hA|l`oZ~i)&2T?NAIO5ZaX#Q6JfUZqKUKY=m9DcTV_Vm(l*t%eBB-ds@uTq9|f2&c~);YwzL(vF!C0141VauWKC%kjqz8 z8oPG>NM$|s+2hPo*PW{AwKa)J$2u))YSTp*uk}C4@DGsKFKiB)gr#}*TiLYix3aQw zBCjqJk2_4VxYqbRx=V&$=aQv}Df((xwz~I#0h#h~ zW0TtBHlf42Q|X^ZjXur=JJ|?$_IiYcWMLU1CHo0b{}$5|s<~0$S)N+=E)r*EU+A>f zXam{76u)PY&x#LrjSC-O*Y?}A=(^~=HaXz)v_0vt)`MZofhW0*>Nx%S(^6ACoA)g! z)Si^AkJx(XxN|`^V++_-HC5NtnRoDU)w6(rClzd!#5+!lVV%K6TLq(?vDpL1R7{-H zJ5~nw_%?s*(t?G3TO>Ur$7(JOJgTX$os4Uk-?ju6wh!-Job4H({jvl~#U3rRCb!Mp zVLe{_oIrkeN5NdFc)Fr;^6l!oPLwfw!k+Ojv-8`&Ep`Z+ym?sB7nR}0zTZZQY0{My z*(mt<{_Co)rUX{?KW;YU2yIufO>|c8`z%y$FJ%AjT`%LQvaHP(touT4R<-Bpu@Tt> z&$llo?fU*SRA#!UHQkUA&D~|YTDnh8^@20KQ=Z~qu8~!lRTEL>axgV=uxS6loQ6|> zWZamoN+o}GP~Gb=e%_CL%GNV`{PCk>eXPjd4-ej^#Nzp4LssS@q;8#pq0cMS+6Z*z z;Nh#QPG;;7;jM%FwT3rZN9G?Jb?eZmo$RT8&TCS8$2 literal 0 HcmV?d00001 diff --git a/zerver/tests/test_thumbnail.py b/zerver/tests/test_thumbnail.py index f49f2ceb18..16d50e8b2b 100644 --- a/zerver/tests/test_thumbnail.py +++ b/zerver/tests/test_thumbnail.py @@ -398,6 +398,56 @@ class TestStoreThumbnail(ZulipTestCase): ), ) + def test_image_orientation(self) -> None: + self.login_user(self.example_user("hamlet")) + + with ( + self.thumbnail_formats(ThumbnailFormat("webp", 100, 75, animated=False)), + self.captureOnCommitCallbacks(execute=True), + ): + with get_test_image_file("orientation.jpg") as image_file: + response = self.assert_json_success( + self.client_post("/json/user_uploads", {"file": image_file}) + ) + path_id = re.sub(r"/user_uploads/", "", response["url"]) + self.assertEqual(Attachment.objects.filter(path_id=path_id).count(), 1) + + image_attachment = ImageAttachment.objects.get(path_id=path_id) + # The bytes in this image are 100 wide, and 600 tall -- + # however, it has EXIF orientation information which says + # to rotate it 270 degrees counter-clockwise. + self.assertEqual(image_attachment.original_height_px, 100) + self.assertEqual(image_attachment.original_width_px, 600) + + # The worker triggers when we exit this block and call the pending callbacks + image_attachment = ImageAttachment.objects.get(path_id=path_id) + self.assert_length(image_attachment.thumbnail_metadata, 1) + generated_thumbnail = StoredThumbnailFormat(**image_attachment.thumbnail_metadata[0]) + + # The uploaded original content is technically "tall", not "wide", with a 270 CCW rotation set. + with BytesIO() as fh: + save_attachment_contents(path_id, fh) + thumbnailed_bytes = fh.getvalue() + with pyvips.Image.new_from_buffer(thumbnailed_bytes, "") as thumbnailed_image: + self.assertEqual(thumbnailed_image.get("vips-loader"), "jpegload_buffer") + self.assertEqual(thumbnailed_image.width, 100) + self.assertEqual(thumbnailed_image.height, 600) + self.assertEqual(thumbnailed_image.get("orientation"), 8) # 270 CCW rotation + + # The generated thumbnail should be wide, not tall, with the default orientation + self.assertEqual(str(generated_thumbnail), "100x75.webp") + self.assertEqual(generated_thumbnail.width, 100) + self.assertEqual(generated_thumbnail.height, 17) + + with BytesIO() as fh: + save_attachment_contents(f"thumbnail/{path_id}/100x75.webp", fh) + thumbnailed_bytes = fh.getvalue() + with pyvips.Image.new_from_buffer(thumbnailed_bytes, "") as thumbnailed_image: + self.assertEqual(thumbnailed_image.get("vips-loader"), "webpload_buffer") + self.assertEqual(thumbnailed_image.width, 100) + self.assertEqual(thumbnailed_image.height, 17) + self.assertEqual(thumbnailed_image.get("orientation"), 1) + def test_big_upload(self) -> None: # We decline to treat as an image a large single-frame image self.login_user(self.example_user("hamlet"))