From 93f64f9e22e69a52f82534a5f515c534b7902153 Mon Sep 17 00:00:00 2001
From: Scott Kidder <scott.kidder@discordapp.com>
Date: Thu, 7 Nov 2024 10:50:55 -0800
Subject: [PATCH] fix: improve GIF loop count detection and add test coverage

- Enhance loop count detection to handle multiple extension blocks
- Add proper cleanup and error handling in GIF parsing
- Add test cases for duplicate loop counts and multiple extensions
---
 giflib.cpp                             |  61 +++++++++++++++++--------
 testdata/dispose_bgnd.gif              | Bin 0 -> 1450 bytes
 testdata/duplicate_number_of_loops.gif | Bin 0 -> 1622 bytes
 webp_test.go                           |  20 ++++++++
 4 files changed, 62 insertions(+), 19 deletions(-)
 create mode 100644 testdata/dispose_bgnd.gif
 create mode 100644 testdata/duplicate_number_of_loops.gif

diff --git a/giflib.cpp b/giflib.cpp
index b10efda0..64504af9 100644
--- a/giflib.cpp
+++ b/giflib.cpp
@@ -1134,32 +1134,55 @@ int giflib_decoder_get_loop_count(const giflib_decoder d) {
         return loop_count;
     }
 
-    // Read extension blocks
+    // Read all blocks until we find NETSCAPE2.0 or hit end
     GifRecordType recordType;
-    if (DGifGetRecordType(gif, &recordType) == GIF_OK && 
-        recordType == EXTENSION_RECORD_TYPE) {
-
-        GifByteType* ExtData;
-        int ExtFunction;
-
-        if (DGifGetExtension(gif, &ExtFunction, &ExtData) == GIF_OK && ExtData != NULL) {
-            // Look for NETSCAPE2.0 extension
-            if (ExtFunction == APPLICATION_EXT_FUNC_CODE && ExtData[0] >= 11 &&
-                memcmp(ExtData + 1, "NETSCAPE2.0", 11) == 0) {
-                // Get the next block with loop count
-                if (DGifGetExtensionNext(gif, &ExtData) == GIF_OK && 
-                    ExtData != NULL && 
-                    ExtData[0] >= 3 && 
-                    ExtData[1] == 1) {
-
-                    loop_count = ExtData[2] | (ExtData[3] << 8);
+    while (DGifGetRecordType(gif, &recordType) == GIF_OK) {
+        switch (recordType) {
+            case EXTENSION_RECORD_TYPE: {
+                GifByteType* ExtData;
+                int ExtFunction;
+                
+                if (DGifGetExtension(gif, &ExtFunction, &ExtData) == GIF_OK && ExtData != NULL) {
+                    // Look for NETSCAPE2.0 extension
+                    if (ExtFunction == APPLICATION_EXT_FUNC_CODE && ExtData[0] >= 11 &&
+                        memcmp(ExtData + 1, "NETSCAPE2.0", 11) == 0) {
+                        // Get the next block with loop count
+                        if (DGifGetExtensionNext(gif, &ExtData) == GIF_OK && 
+                            ExtData != NULL && 
+                            ExtData[0] >= 3 && 
+                            ExtData[1] == 1) {
+                            loop_count = ExtData[2] | (ExtData[3] << 8);
+                            goto cleanup; // Found what we need
+                        }
+                    }
+                    
+                    // Skip any remaining extension blocks
+                    while (ExtData != NULL) {
+                        if (DGifGetExtensionNext(gif, &ExtData) != GIF_OK) {
+                            goto cleanup;
+                        }
+                    }
                 }
+                break;
             }
+            
+            case IMAGE_DESC_RECORD_TYPE:
+                // Skip image data
+                if (DGifGetImageDesc(gif) != GIF_OK) {
+                    goto cleanup;
+                }
+                break;
+                
+            case TERMINATE_RECORD_TYPE:
+                goto cleanup;
+                
+            default:
+                break;
         }
     }
 
+cleanup:
     DGifCloseFile(gif, &error);
     delete loopReader;
-
     return loop_count;
 }
diff --git a/testdata/dispose_bgnd.gif b/testdata/dispose_bgnd.gif
new file mode 100644
index 0000000000000000000000000000000000000000..d68504010fe1248b656d15ee84172da4ab402219
GIT binary patch
literal 1450
zcmYk*dpOg390%~<GHf`cLs{lB$}MSI67eXvxjop4h$3=5ZcT2RTbpIBhs|x6o~X@j
zF1faiJaIfkR4kFi33ISGr4vbqGj%R~zt8uN@Avch=lgozA6t8jv58**5CFUb09ER4
z`V%2y3k(hb006No?QG-bYGr}7K^q{&K%yso(Gj(Zs{3|w@@gNi%H1t*o62sT>Drc~
z%-Iz$W>&3$+SY`PXz7U!Q~KtU``ig>i5p6lQFeuJ`lA3cRA6#wv@Ry}Ug$M`1ATit
zD~#I=yMKnAT-&ZJ$YspM9QezeH({0VrMhWTk~4r(;`mLbmn?bo;qmVwo`~9zpG-_h
zFKf7ius2&!c?XDt*JoVTN`+qabKVPR;>H<vP^hD68JjT_M%mt0)bFyn93GnkXLSuf
zf13nWId16m+CTY$D?8%IEANpXa*vyLdq+lE(NaE+$XabRs-94b_Ppkj_s7NCB)N*p
zE9VGllOU9K2pp1@)Fz1fgq(s0<EXVdTJs(-gG<&yv52&z+0|5t9?k9D(h;(SUNSed
zbQLyi=M|(C9(=O;$t|<Q&tIU1OPTus5h`R4RbL#C0@OvDyhpja3lM=84ULNP$9MNG
zfI+s$?%vv~?KOP2z-!n+d-elk@`%L_KMv4Tr@}*MEL=`%VsYKgd{mj*HnjLI*$+^c
zm64bpT<FQb*!r)R=xaocNM(f_^{M5vPnnV&(uqQZI)*ZM;I834=(CBqGusigg10x~
z2?<17oD{}_h}R?ogd6ED`XEgx6b#Uzq+5pyB<ZE}vT{~=Sh|r#DVW(*-q>7qxS>JZ
zS{xAPF&dTCfl@IHhs)_=)g4RW!5?+<P8ng8Aq=M27~iNDJUhvF+%qe9Gp{$Z$Xr?$
zv^<=47OwJDbl06XKW-ql)I2|pPiiBTP^ZN~s`cwGYTa`3qWt|!rU|HiB})})h_Jf5
zyG1r4Bpi|Y%}4B&F>iVYnG2kwPLedNL7qxD@Eh;S16V~VNT4n;G|)uo{&rKD(KfUe
z&5lTEg%17+3-@h)ns#0%`3}8W((p5={-9c=B`3GFV=(@*3fXf&&VFT~(QzRP6)fET
zh)dCtcQbSLk2NDJxuJ{_A_8rcY$Fq>#ZItzyh3mh9uXgN#UkFYFv5lvinWI+U{j2+
zF&bCT<|k+0tKfF%N-4J%mGwAd^Go`BesQ<HFg)VolQkfd<oIx+_ktpFY?@_VIW1#}
z7c9Q!tq8Apgw+T=7R0+2gMNiU|93D4(EmAz{x6!=ZwTrwb>HG5wj**d2fCnwEO!tB
zJr0)uTdQAxJrI7$w%fWBHuHR}J~Nhxc%}y#B8v6l%w?%9R=S!9ZPa>3p_kRvNz3wn
zwEV@UOA6j7*2yCkcG&?~RT+0GAVo?|Rgk-Fg*hQkjw4Vk&3?X$29tB+@HBLEmWONZ
z%`lo4BBC&fUP7hBA_$3DH!JhXC}@WC9d>Cvvw%TpF)WE~z&A0ud94rH^Q$;r{_%IZ
z%T*XO-f(?C;Xb9eu5(0_@o<zsw10FGKNZ`THj5ZtdOYtpU@$nB=iB}pbHh#6Y3602
z)A9h*8%c6Z^@VKY`ckD|tc~M2SNqyC71+jZG|+dUkpfPC#UlL^7uMc<?c?O+88?s2
z_B;IOrR`r_dzS1Cm-MOXU=>xD&v{WMYa+wbIzJ$Uu`8e$iC$@64D(%#%wcMK4$7WC
zk>@27kloi$d%v1mH0D#a1;?;ytgSS6m5%8Br*obLI0_2*m|$G+?-w8)>slY^TG-%c
zIuH$CW2DK8vFeQL2WtXt@k&!g1<S7UrQ<q<bw}M_DoSvRGY$?cFA<i@4z2V{G~+AE
cl##=y2ny1U8=ZrrCOSjMj9z_l1pw2(0p$3GlmGw#

literal 0
HcmV?d00001

diff --git a/testdata/duplicate_number_of_loops.gif b/testdata/duplicate_number_of_loops.gif
new file mode 100644
index 0000000000000000000000000000000000000000..ac315ee99f312dec957aaa08cc3aa065acad4993
GIT binary patch
literal 1622
zcmZ?wbhEHbWMp7uXlED&qaiS&LqPFAx1VcBu(M-;tC5}oGb0lNgAOP_K-q(VgN1>S
ag@plK4KtFO_WvU~_(nZH(nA26N2~$y{0yQ1

literal 0
HcmV?d00001

diff --git a/webp_test.go b/webp_test.go
index 2240eb9f..55588f95 100644
--- a/webp_test.go
+++ b/webp_test.go
@@ -413,6 +413,26 @@ func testNewWebpEncoderWithAnimatedGIFSource(t *testing.T) {
 			resizeMethod: ImageOpsResize,
 			wantLoops:    1,
 		},
+		{
+			name:         "Animated GIF with duplicate number of loop count, use the first loop count",
+			inputPath:    "testdata/duplicate_number_of_loops.gif",
+			outputPath:   "testdata/out/duplicate_number_of_loops.webp",
+			width:        200,
+			height:       200,
+			quality:      80,
+			resizeMethod: ImageOpsResize,
+			wantLoops:    2,
+		},
+		{
+			name:         "Animated GIF with multiple extension blocks",
+			inputPath:    "testdata/dispose_bgnd.gif",
+			outputPath:   "testdata/out/dispose_bgnd.webp",
+			width:        200,
+			height:       200,
+			quality:      80,
+			resizeMethod: ImageOpsResize,
+			wantLoops:    0,
+		},
 	}
 
 	for _, tc := range testCases {