From 79b947e15b5871d9956cd82cbe59b9edddf27c1d Mon Sep 17 00:00:00 2001
From: Aliaksandr Rahalevich <saksmlz@gmail.com>
Date: Wed, 22 Nov 2023 13:05:39 -0800
Subject: [PATCH 1/5] add failing test

---
 tests/biff5_write.xls | Bin 0 -> 10752 bytes
 tests/test.rs         |  20 ++++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 tests/biff5_write.xls

diff --git a/tests/biff5_write.xls b/tests/biff5_write.xls
new file mode 100644
index 0000000000000000000000000000000000000000..cb9a15589b2a4182b4f44c2ad73779f9c6879170
GIT binary patch
literal 10752
zcmeHNeQX@X6@R<;>96>lPsr!u#EI?L#=dhxAR*@@CB(%^fCNPqLD9v&BPPZdE@x^g
zLM|<!R-!buQzMtO6huS`l+TDD6&h1j@Xt}I3PcHk28d8wsulvM1wppIH*<TpyEncB
zkdRU__Uz5Pee-^A=Dm44GrNC!spj~-k57M#TxmIZ=#*JTej8mwnX^bCq8b#qe#$gW
zNhCt?n{*b9z~|6)5{&0T_9FX`+2#S{AaWUUIdTPZC2|$=1mtRDxPY)d+uzD(QP;mw
z6(evZZ1`1@4v(=0Qiq^Wkgr5z;!3OKkG%dmv*70YUw=uP$W_TkD=CS;?RK^=R4yl7
zV|?~q{yme%4E_6AlG8^UXn?NAUyM?86LnHQC7@ACP}0<ka(@Y-^}y#C$n)n+@mbs$
zUyywCEg*CY9pXSre?fhg<oeIpoTSv!SJw5h97|acv)t#SMe%IlEzq75hB<=L81mc5
zn`sA;@%5&a&?2;8Kdv=&uFz<rxK0w+i-|qxX^na|wk$!3pF$sL9pc`P#kE#kC*Z2l
zVU6v5R8ZPzD!q?dkoL0Cbb33mD%RJ#HP!od-q{9^SwhdST2h1Sp+Jl^Xma1l>*gKz
z&N1}SAuZpAds<`ll-xa?Fz%gY<K9_5&K-@SR)1VL8nusehj1rng`zfP<F+Xur;Se2
z#<`=x+PSfI;qcSVt-);Xbj-<VwexD{HqD*ex}{<1=6b8LxnZ_<4xWxPEduUdePz7o
z8Yhc^hvzJdrKQy=19cWt2c>GAJT6H&#zoXf;VrB}qhV>3t8T6rgv||0bA;OFT4=PT
zzGDjzny7&qw$$5OH5&FNjfR8&Vrrs`>_Ad;Czepk1{aMM0NZIHwdDb=)_F{;semoC
zJ1$lcL?4?5jTvxQB&PB71nr~K(RdQM`BXHP>*hOKX&M<``?&C$4ShTGBZEgxj-N$)
zR6)B(K~uGTP{F5?_wX6=sc;`rv{lJ_(#AI>*WX_%XunX<?6_9j5e47l3YxvYR@>hx
z_=XiU74H2CJ{7LLAH4Qv`~Jq(me=2)?He&xv~;%IRl#$H_hU>uZ0y<n)b1m5nC8Q*
z1ns|0e`!$AniRCgA{y@%AkQ%`d4B-fp@TO4c>e&}%VS2zE)>O_=KVyD)@;#ue<5j^
zWiFZngZCZa+xMugzoA0gUaKwdQNVZT=Qdq(b7B8O#)_uSrm14Acn`z!Fn+gm-eQea
z8PgnN<(GU8T_#H!`j*+H-7NdYD-yJI@bON`K39kJ5G+y^Zmlz#jTPP9@&0t%P;e~`
z@xbJg=ACU;QJ7k(jtpu>UV+tNH_5eO8@N1>SxEtJJaO#Z4@@7I<+L~JU~Qd2YMdb9
zVvyPrAmIXtFP=Dg>~%HBL?=k37-W74kVpZ<A5XmZ!JBH3NluXVVvxoXAngT^Ks<5c
zum4noOm>1SDh6pO0kWt75{xH~{_8_E$P_2Y;$o0+36RAFkg|B<^S{5Q2AN7#?WMhI
z4nW$^z`L?tPiIf4L8duD6y7zr1c=?cvR&^!eOL{0juS-TT?<Np*u5*;^_LURs6ozk
zf+)OeVF?hscV)Y>V@K5>)14p+?>fH(h~2xgU5DTNn;K+>6GY)%krE(w@5*+4_R1kO
z$V{qQT-v(~fGj=(?+QnL_T;M)1dh%7)_m-m<pfc9*Sr!ScJB&Dj*je8gUoh<D7>q_
z1c=?c!jV6`dbb*6juS-TT}>rG?A{fQJpQW})F6fvMB!bnB|z-n6^;zOcuWm)o)bjj
zUF{`6?A{fQe0t<3Dv)xjr9l3fxWe^JoMi^aeGU?>-}9_m<XlT6yvQx`JeNqj=iu<;
z^0jJ_^DL3@8Mnwfmq@#x5Z(RT@2ExASt8*fZjlWxk#?^j%Jkf)7CGM%34d^lY<7vX
z`vB2p4?nLKxxf+$>$^p^xkTD6PV}FV)hdyGswdCNSkF-KLZ`L08Kwv4f-&mJKlFS!
z*D_!DqY%U8{p7bI#suthD{zR@J<u0R_a^&?f_t3e^r>bAt%A5dam?0-QsG!HAQn?B
z^5n^rdGVj_A}QXF6&&B(y?eK*QNU3PI%;K(+OS@QeS7(;ZSn5y#;SN<-%xP3ORH&S
zIkeh_g<Us}KMj=Q(6A3(F6eaNz=7hCvvi6^qozNvldM@f$(p5;tPSg4TA1;eLqWr-
ziq8X(RXBx|r=ZY)n71k82XDQL*N=BX1zv-$Mzp_%n}{~e886z*ceU}FL9`jbnQBU`
zW4m$AYPD#|1kj?z)goY47PW9<O``KDbjje3zF2=OolM<iY>W@4hk}<n1?h{-5M4rp
zIKpJR_Y2$e;qFZs@r_Vs5Wc{taJ-DR44r!v1<*6^rO6~5bg5ISHXX;!mtw$@E*E|N
z-vt*1l;xt8)JA1%;<29I{%Z}a_+3hRLKxFEP=-Ce7fN0W<>X3VtnP7KJEwD(6vi$X
zz*)BpdBb`owhDRCh;nYxH44!((v2h9j6QIL*WxTXQV@-3=N3ItAzDVfaYW09zAi+!
z7epiKxdMP^l0q~`K53_beL#lwAT1(aXa9}?n18@&D19<|!YBGW;C*c9!qhUeqhe~Y
z(WB8Dzv^_KfMribS8xCJcu#J=U*=Nf97LfmVczX9b&q4ld-DF5k33eaF*6!w`LJG2
z7f@hbd?1~QA(*%Vur?C`cpb!YFH`U%4kL1($o(_l+n?|2{&?Ok;YJz4<cl~p)d+y=
z;M?*Q2#d;%yM`j9<>$u|@~%s!<3qu%P6>KB<_AkiLnVI2!5dbgh?#|K8B(8k;t5+w
zl{G7T$(j|uWNlb4qYKHuejqI@{aqL26bN1~`d@5oduZoaC^}&2!GuC+mo-ZdS+n$z
zH4cpzWqcU7c}^94J{aM0^f;B@uJdrQgDxfC#@=)vMyk!(##0T$Y=l>&VKN@8Vh`6?
zz%mF(rrGkY(*n=ALI4l7w7OTglH){xduqiD*;A{=j#W(;Q{|>us-J_C45Cq|l6nQA
z@g~SmVJ2DeNfkWV)x_8i;l+`BF~e`9-U#|>&f&@E-OKdZhdw$Pyp`yof6!hYo*q85
z+Dm(HxCrkG55OZcG&VMdpA2;$JVkFbhspf-4qEs2Cc3u9q*D*9rrS4cqWhlRd)oVy
zHSy0mi!H_|jiPRjW(hN&-g)O8iX=>mWV4_{Z_cC038F+|ltxBI2n-Y%1p!elf;=iR
zXpvFm2;GKpy<!G;BCugwJf8mc20jQAFVy@Y@j|TvXE8E)sca_E6?o}b1sn5x&=CWw
zMY#r7!og6GK6~(kKd;?-d31}QHwyYfas9raWWZykI`|>sh0z`4g*=V+fa5tYkMb1c
zABa}BitD{tS~G|I^U~m*pB#}=jz_KE_B*cvZ#nIPW}FWC`GC<Y>fG*NpitMrd9b`t
z_aM|9oGj~vKqWsUVC{mr{$nvAV~18PXk2mAcnsV$eo<+sJxk|+B0oNOKgx*I!Pj61
z<_)F^%dN&$mNoH_f)^gHLBnbU|D9~6>vnqe3Uq;4J(Nf$WtF)-v^AFU==|PLr{#1N
zQsn9r9g=}Xc&N1{!d2Ex@_$cOby2-xVDMFp;77Q-jAr8KHHz6a5jFSA%H<fRr}%79
zq*<b|D9Uld{D9F6dY`w(B@z$zS4)h?W5M5Vu*vwWl*@d6%fH{G|L+kvg`n*zx4aHU
z&n2cDzwMrrpRC^&dg!M?TDaiJ*IGg2pv`{E_c_FKK<YqdC+|Y$@Ua7#&+BnCPLq&-
zh|E3~T0k@#zwBq6xQwVsYkRv>$(_kW+E^Rw?(I(}cWyH-8SIYt;a&&wV;gSV&fkr2
z`S^1A4GB*`PNnqMUwQGB{Vg*>!*@adrcWMW{WHky<E%f|`P~ZlBZS<6%zCdu=6-BO
zw)AE`PS)>iG+-Ir|6KH^O3;{}`^9{llBw+?<m?pjg4v6bh92+(fxXQA?aVxI7sf`6
zh;iWscy|zPZEZL5(#1Sovbu96zta$0EkdIf-r1inX}R6p4y>NG(sid}FS^g)W!Ps(
a5rcxj{<Bp<|FWbnivMMC#%p&r<NptqCD6bC

literal 0
HcmV?d00001

diff --git a/tests/test.rs b/tests/test.rs
index 26166f33..389608d2 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -1488,3 +1488,23 @@ fn any_sheets_ods() {
         ]
     );
 }
+
+#[test]
+fn text_encoding_issue() {
+    let path = format!("{}/tests/biff5_write.xls", env!("CARGO_MANIFEST_DIR"));
+    let mut workbook: Xls<_> = open_workbook(path).unwrap();
+
+    let first_sheet_name = workbook.sheet_names().first().unwrap().to_owned();
+
+    assert_eq!("SheetJS", first_sheet_name);
+
+    let range = workbook
+        .worksheet_range(&first_sheet_name)
+        .unwrap()
+        .unwrap();
+
+    let second_row = range.rows().into_iter().nth(1).unwrap();
+    let cell_text = second_row.get(3).unwrap().to_string();
+
+    assert_eq!("sheetjs", cell_text);
+}

From b0c868e4afca5eaee7e428348ff8b3e80cc3e961 Mon Sep 17 00:00:00 2001
From: Aliaksandr Rahalevich <saksmlz@gmail.com>
Date: Wed, 22 Nov 2023 16:28:53 -0800
Subject: [PATCH 2/5] use BIFF version to decode strings

---
 src/xls.rs | 130 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 116 insertions(+), 14 deletions(-)

diff --git a/src/xls.rs b/src/xls.rs
index 80ed0dab..8c187dba 100644
--- a/src/xls.rs
+++ b/src/xls.rs
@@ -261,6 +261,7 @@ impl<RS: Read + Seek> Xls<RS> {
         let mut xtis = Vec::new();
         let mut formats = BTreeMap::new();
         let mut xfs = Vec::new();
+        let mut biff = 8; // BIFF version
         let codepage = self.options.force_codepage.unwrap_or(1200);
         let mut encoding = XlsEncoding::from_codepage(codepage)?;
         #[cfg(feature = "picture")]
@@ -300,10 +301,15 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     // RRTabId
                     0x0085 => {
-                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding)?;
+                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding, biff)?;
                         self.metadata.sheets.push(sheet.clone());
                         sheet_names.push((pos, sheet.name)); // BoundSheet8
                     }
+                    // BOF
+                    0x0809 => {
+                        let bof = parse_bof(&mut r)?;
+                        biff = bof.biff;
+                    }
                     0x0018 => {
                         // Lbl for defined_names
                         let cch = r.data[3] as usize;
@@ -387,11 +393,11 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     //0x0201 => cells.push(parse_blank(r.data)?), // 513: Blank
                     0x0203 => cells.push(parse_number(r.data, &self.formats, self.is_1904)?), // 515: Number
-                    0x0204 => cells.extend(parse_label(r.data, &encoding)?), // 516: Label [MS-XLS 2.4.148]
-                    0x0205 => cells.push(parse_bool_err(r.data)?),           // 517: BoolErr
+                    0x0204 => cells.extend(parse_label(r.data, &encoding, biff)?), // 516: Label [MS-XLS 2.4.148]
+                    0x0205 => cells.push(parse_bool_err(r.data)?),                 // 517: BoolErr
                     0x0207 => {
                         // 519 String (formula value)
-                        let val = DataType::String(parse_string(r.data, &encoding)?);
+                        let val = DataType::String(parse_string(r.data, &encoding, biff)?);
                         cells.push(Cell::new(fmla_pos, val))
                     }
                     0x027E => cells.push(parse_rk(r.data, &self.formats, self.is_1904)?), // 638: Rk
@@ -455,10 +461,85 @@ impl<RS: Read + Seek> Xls<RS> {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct Bof {
+    dt: u16,
+    biff_version: u16,
+    biff: u8,
+    guess: bool,
+}
+
+impl Default for Bof {
+    fn default() -> Self {
+        Self {
+            dt: 0,
+            biff: 8,
+            biff_version: 0x0600,
+            guess: false,
+        }
+    }
+}
+
+/// [MS-XLS] 2.4.21
+fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
+    let mut dt = 0;
+    let biff_version = read_u16(&r.data[..2]);
+    r.data = &r.data[2..];
+
+    if r.data.len() >= 2 {
+        dt = read_u16(r.data);
+        r.data = &r.data[2..];
+    };
+
+    let mut biff = match biff_version {
+        // BIFF8
+        0x0600 => 8,
+        // BIFF5
+        0x0500 => 5,
+        // BIFF4
+        0x0400 => 4,
+        // BIFF3
+        0x0300 => 3,
+        // BIFF2
+        0x0200 | 0x0002 | 0x0007 => 2,
+        _ => {
+            if r.data.len() > 6 {
+                return Err(XlsError::Unrecognized {
+                    typ: "Unexpected BIFF Ver",
+                    val: 0, // TODO: return biff version
+                });
+            } else {
+                8
+            }
+        }
+    };
+
+    let guess = biff_version == 0;
+
+    if biff_version == 0 && dt == 0x1000 {
+        biff = 5;
+    }
+    if biff == 8 && biff_version == 0 && dt == 16 {
+        biff = 2;
+    }
+
+    let bof = Bof {
+        biff,
+        dt,
+        biff_version,
+        guess,
+    };
+
+    r.data = &r.data[..r.data.len()];
+
+    Ok(bof)
+}
+
 /// BoundSheet8 [MS-XLS 2.4.28]
 fn parse_sheet_metadata(
     r: &mut Record<'_>,
     encoding: &XlsEncoding,
+    biff: u8,
 ) -> Result<(usize, Sheet), XlsError> {
     let pos = read_u32(r.data) as usize;
     let visible = match r.data[4] & 0b0011_1111 {
@@ -485,7 +566,7 @@ fn parse_sheet_metadata(
         }
     };
     r.data = &r.data[6..];
-    let name = parse_short_string(r, encoding)?;
+    let name = parse_short_string(r, encoding, biff)?;
     let sheet_name = name
         .as_bytes()
         .iter()
@@ -628,7 +709,11 @@ fn rk_num(rk: &[u8], formats: &[CellFormat], is_1904: bool) -> DataType {
 }
 
 /// ShortXLUnicodeString [MS-XLS 2.5.240]
-fn parse_short_string(r: &mut Record<'_>, encoding: &XlsEncoding) -> Result<String, XlsError> {
+fn parse_short_string(
+    r: &mut Record<'_>,
+    encoding: &XlsEncoding,
+    biff: u8,
+) -> Result<String, XlsError> {
     if r.data.len() < 2 {
         return Err(XlsError::Len {
             typ: "short string",
@@ -636,16 +721,23 @@ fn parse_short_string(r: &mut Record<'_>, encoding: &XlsEncoding) -> Result<Stri
             found: r.data.len(),
         });
     }
+
     let cch = r.data[0] as usize;
-    let high_byte = r.data[1] & 0x1 != 0;
-    r.data = &r.data[2..];
+    r.data = &r.data[1..];
+    let mut high_byte = None;
+
+    if biff == 8 {
+        high_byte = Some(r.data[0] & 0x1 != 0);
+        r.data = &r.data[1..];
+    }
+
     let mut s = String::with_capacity(cch);
-    let _ = encoding.decode_to(r.data, cch, &mut s, Some(high_byte));
+    let _ = encoding.decode_to(r.data, cch, &mut s, high_byte);
     Ok(s)
 }
 
 /// XLUnicodeString [MS-XLS 2.5.294]
-fn parse_string(r: &[u8], encoding: &XlsEncoding) -> Result<String, XlsError> {
+fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: u8) -> Result<String, XlsError> {
     if r.len() < 4 {
         return Err(XlsError::Len {
             typ: "string",
@@ -654,13 +746,23 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding) -> Result<String, XlsError> {
         });
     }
     let cch = read_u16(r) as usize;
-    let high_byte = r[2] & 0x1 != 0;
+
+    let (high_byte, start) = if (biff >= 2 && biff <= 5) || biff >= 12 {
+        (None, 2)
+    } else {
+        (Some(r[2] & 0x1 != 0), 3)
+    };
+
     let mut s = String::with_capacity(cch);
-    let _ = encoding.decode_to(&r[3..], cch, &mut s, Some(high_byte));
+    let _ = encoding.decode_to(&r[start..], cch, &mut s, high_byte);
     Ok(s)
 }
 
-fn parse_label(r: &[u8], encoding: &XlsEncoding) -> Result<Option<Cell<DataType>>, XlsError> {
+fn parse_label(
+    r: &[u8],
+    encoding: &XlsEncoding,
+    biff: u8,
+) -> Result<Option<Cell<DataType>>, XlsError> {
     if r.len() < 6 {
         return Err(XlsError::Len {
             typ: "label",
@@ -673,7 +775,7 @@ fn parse_label(r: &[u8], encoding: &XlsEncoding) -> Result<Option<Cell<DataType>
     let _ixfe = read_u16(&r[4..]);
     return Ok(Some(Cell::new(
         (row as u32, col as u32),
-        DataType::String(parse_string(&r[6..], encoding)?),
+        DataType::String(parse_string(&r[6..], encoding, biff)?),
     )));
 }
 

From 6da12738984569b57a0d274bfe15e8ac923078fa Mon Sep 17 00:00:00 2001
From: Aliaksandr Rahalevich <saksmlz@gmail.com>
Date: Wed, 22 Nov 2023 16:48:45 -0800
Subject: [PATCH 3/5] cleanup & add comments

---
 src/xls.rs    | 51 +++++++++++++--------------------------------------
 tests/test.rs |  4 ++--
 2 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/src/xls.rs b/src/xls.rs
index 8c187dba..15e41387 100644
--- a/src/xls.rs
+++ b/src/xls.rs
@@ -261,7 +261,7 @@ impl<RS: Read + Seek> Xls<RS> {
         let mut xtis = Vec::new();
         let mut formats = BTreeMap::new();
         let mut xfs = Vec::new();
-        let mut biff = 8; // BIFF version
+        let mut biff = 8; // Binary Interchange File Format (BIFF) version
         let codepage = self.options.force_codepage.unwrap_or(1200);
         let mut encoding = XlsEncoding::from_codepage(codepage)?;
         #[cfg(feature = "picture")]
@@ -461,34 +461,19 @@ impl<RS: Read + Seek> Xls<RS> {
     }
 }
 
-#[derive(Debug)]
-pub(crate) struct Bof {
-    dt: u16,
-    biff_version: u16,
+/// https://learn.microsoft.com/en-us/openspecs/office_file_formats/ms-xls/4d6a3d1e-d7c5-405f-bbae-d01e9cb79366
+struct Bof {
+    /// Binary Interchange File Format
     biff: u8,
-    guess: bool,
-}
-
-impl Default for Bof {
-    fn default() -> Self {
-        Self {
-            dt: 0,
-            biff: 8,
-            biff_version: 0x0600,
-            guess: false,
-        }
-    }
 }
 
-/// [MS-XLS] 2.4.21
+/// BOF [MS-XLS] 2.4.21
 fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
     let mut dt = 0;
     let biff_version = read_u16(&r.data[..2]);
-    r.data = &r.data[2..];
 
     if r.data.len() >= 2 {
-        dt = read_u16(r.data);
-        r.data = &r.data[2..];
+        dt = read_u16(&r.data[2..]);
     };
 
     let mut biff = match biff_version {
@@ -503,10 +488,11 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
         // BIFF2
         0x0200 | 0x0002 | 0x0007 => 2,
         _ => {
-            if r.data.len() > 6 {
-                return Err(XlsError::Unrecognized {
-                    typ: "Unexpected BIFF Ver",
-                    val: 0, // TODO: return biff version
+            if r.data[4..].len() > 6 {
+                return Err(XlsError::Len {
+                    typ: "BOF data length",
+                    expected: 6,
+                    found: r.data.len(),
                 });
             } else {
                 8
@@ -514,8 +500,6 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
         }
     };
 
-    let guess = biff_version == 0;
-
     if biff_version == 0 && dt == 0x1000 {
         biff = 5;
     }
@@ -523,16 +507,7 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
         biff = 2;
     }
 
-    let bof = Bof {
-        biff,
-        dt,
-        biff_version,
-        guess,
-    };
-
-    r.data = &r.data[..r.data.len()];
-
-    Ok(bof)
+    Ok(Bof { biff })
 }
 
 /// BoundSheet8 [MS-XLS 2.4.28]
@@ -747,7 +722,7 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: u8) -> Result<String, Xl
     }
     let cch = read_u16(r) as usize;
 
-    let (high_byte, start) = if (biff >= 2 && biff <= 5) || biff >= 12 {
+    let (high_byte, start) = if (2..=5).contains(&biff) || biff >= 12 {
         (None, 2)
     } else {
         (Some(r[2] & 0x1 != 0), 3)
diff --git a/tests/test.rs b/tests/test.rs
index 389608d2..8477f9c9 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -1490,7 +1490,7 @@ fn any_sheets_ods() {
 }
 
 #[test]
-fn text_encoding_issue() {
+fn issue_374() {
     let path = format!("{}/tests/biff5_write.xls", env!("CARGO_MANIFEST_DIR"));
     let mut workbook: Xls<_> = open_workbook(path).unwrap();
 
@@ -1503,7 +1503,7 @@ fn text_encoding_issue() {
         .unwrap()
         .unwrap();
 
-    let second_row = range.rows().into_iter().nth(1).unwrap();
+    let second_row = range.rows().nth(1).unwrap();
     let cell_text = second_row.get(3).unwrap().to_string();
 
     assert_eq!("sheetjs", cell_text);

From 8b08c49afe29a6663a13363e4c84a067aa1b44da Mon Sep 17 00:00:00 2001
From: Aliaksandr Rahalevich <saksmlz@gmail.com>
Date: Sun, 26 Nov 2023 08:45:57 -0800
Subject: [PATCH 4/5] Address feedback

---
 src/xls.rs | 79 +++++++++++++++++++++++++-----------------------------
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/src/xls.rs b/src/xls.rs
index 15e41387..00e3bcca 100644
--- a/src/xls.rs
+++ b/src/xls.rs
@@ -261,7 +261,7 @@ impl<RS: Read + Seek> Xls<RS> {
         let mut xtis = Vec::new();
         let mut formats = BTreeMap::new();
         let mut xfs = Vec::new();
-        let mut biff = 8; // Binary Interchange File Format (BIFF) version
+        let mut biff = Biff::Biff8; // Binary Interchange File Format (BIFF) version
         let codepage = self.options.force_codepage.unwrap_or(1200);
         let mut encoding = XlsEncoding::from_codepage(codepage)?;
         #[cfg(feature = "picture")]
@@ -301,7 +301,7 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     // RRTabId
                     0x0085 => {
-                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding, biff)?;
+                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding, &biff)?;
                         self.metadata.sheets.push(sheet.clone());
                         sheet_names.push((pos, sheet.name)); // BoundSheet8
                     }
@@ -393,11 +393,11 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     //0x0201 => cells.push(parse_blank(r.data)?), // 513: Blank
                     0x0203 => cells.push(parse_number(r.data, &self.formats, self.is_1904)?), // 515: Number
-                    0x0204 => cells.extend(parse_label(r.data, &encoding, biff)?), // 516: Label [MS-XLS 2.4.148]
-                    0x0205 => cells.push(parse_bool_err(r.data)?),                 // 517: BoolErr
+                    0x0204 => cells.extend(parse_label(r.data, &encoding, &biff)?), // 516: Label [MS-XLS 2.4.148]
+                    0x0205 => cells.push(parse_bool_err(r.data)?),                  // 517: BoolErr
                     0x0207 => {
                         // 519 String (formula value)
-                        let val = DataType::String(parse_string(r.data, &encoding, biff)?);
+                        let val = DataType::String(parse_string(r.data, &encoding, &biff)?);
                         cells.push(Cell::new(fmla_pos, val))
                     }
                     0x027E => cells.push(parse_rk(r.data, &self.formats, self.is_1904)?), // 638: Rk
@@ -464,7 +464,18 @@ impl<RS: Read + Seek> Xls<RS> {
 /// https://learn.microsoft.com/en-us/openspecs/office_file_formats/ms-xls/4d6a3d1e-d7c5-405f-bbae-d01e9cb79366
 struct Bof {
     /// Binary Interchange File Format
-    biff: u8,
+    biff: Biff,
+}
+
+/// https://www.loc.gov/preservation/digital/formats/fdd/fdd000510.shtml#notes
+enum Biff {
+    Biff2,
+    Biff3,
+    Biff4,
+    Biff5,
+    Biff8,
+    // Used by MS-XLSB Workbook(2.1.7.61) or Worksheet(2.1.7.61) which are not supported yet.
+    // Biff12,
 }
 
 /// BOF [MS-XLS] 2.4.21
@@ -472,41 +483,26 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
     let mut dt = 0;
     let biff_version = read_u16(&r.data[..2]);
 
-    if r.data.len() >= 2 {
+    if r.data.len() >= 4 {
         dt = read_u16(&r.data[2..]);
     };
 
-    let mut biff = match biff_version {
-        // BIFF8
-        0x0600 => 8,
-        // BIFF5
-        0x0500 => 5,
-        // BIFF4
-        0x0400 => 4,
-        // BIFF3
-        0x0300 => 3,
-        // BIFF2
-        0x0200 | 0x0002 | 0x0007 => 2,
-        _ => {
-            if r.data[4..].len() > 6 {
-                return Err(XlsError::Len {
-                    typ: "BOF data length",
-                    expected: 6,
-                    found: r.data.len(),
-                });
+    let biff = match biff_version {
+        0x0200 | 0x0002 | 0x0007 => Biff::Biff2,
+        0x0300 => Biff::Biff3,
+        0x0400 => Biff::Biff4,
+        0x0500 => Biff::Biff5,
+        0x0600 => Biff::Biff8,
+        0 => {
+            if dt == 0x1000 {
+                Biff::Biff5
             } else {
-                8
+                Biff::Biff8
             }
         }
+        _ => Biff::Biff8,
     };
 
-    if biff_version == 0 && dt == 0x1000 {
-        biff = 5;
-    }
-    if biff == 8 && biff_version == 0 && dt == 16 {
-        biff = 2;
-    }
-
     Ok(Bof { biff })
 }
 
@@ -514,7 +510,7 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
 fn parse_sheet_metadata(
     r: &mut Record<'_>,
     encoding: &XlsEncoding,
-    biff: u8,
+    biff: &Biff,
 ) -> Result<(usize, Sheet), XlsError> {
     let pos = read_u32(r.data) as usize;
     let visible = match r.data[4] & 0b0011_1111 {
@@ -687,7 +683,7 @@ fn rk_num(rk: &[u8], formats: &[CellFormat], is_1904: bool) -> DataType {
 fn parse_short_string(
     r: &mut Record<'_>,
     encoding: &XlsEncoding,
-    biff: u8,
+    biff: &Biff,
 ) -> Result<String, XlsError> {
     if r.data.len() < 2 {
         return Err(XlsError::Len {
@@ -701,7 +697,7 @@ fn parse_short_string(
     r.data = &r.data[1..];
     let mut high_byte = None;
 
-    if biff == 8 {
+    if matches!(biff, Biff::Biff8) {
         high_byte = Some(r.data[0] & 0x1 != 0);
         r.data = &r.data[1..];
     }
@@ -712,7 +708,7 @@ fn parse_short_string(
 }
 
 /// XLUnicodeString [MS-XLS 2.5.294]
-fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: u8) -> Result<String, XlsError> {
+fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: &Biff) -> Result<String, XlsError> {
     if r.len() < 4 {
         return Err(XlsError::Len {
             typ: "string",
@@ -722,10 +718,9 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: u8) -> Result<String, Xl
     }
     let cch = read_u16(r) as usize;
 
-    let (high_byte, start) = if (2..=5).contains(&biff) || biff >= 12 {
-        (None, 2)
-    } else {
-        (Some(r[2] & 0x1 != 0), 3)
+    let (high_byte, start) = match biff {
+        Biff::Biff2 | Biff::Biff3 | Biff::Biff4 | Biff::Biff5 => (None, 2),
+        _ => (Some(r[2] & 0x1 != 0), 3),
     };
 
     let mut s = String::with_capacity(cch);
@@ -736,7 +731,7 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: u8) -> Result<String, Xl
 fn parse_label(
     r: &[u8],
     encoding: &XlsEncoding,
-    biff: u8,
+    biff: &Biff,
 ) -> Result<Option<Cell<DataType>>, XlsError> {
     if r.len() < 6 {
         return Err(XlsError::Len {

From ba29f727b44ae87fac666484c0f5011b384d2bbd Mon Sep 17 00:00:00 2001
From: Aliaksandr Rahalevich <saksmlz@gmail.com>
Date: Mon, 27 Nov 2023 16:52:32 -0800
Subject: [PATCH 5/5] Address feedback

---
 src/xls.rs | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/xls.rs b/src/xls.rs
index 00e3bcca..2f83677a 100644
--- a/src/xls.rs
+++ b/src/xls.rs
@@ -301,7 +301,7 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     // RRTabId
                     0x0085 => {
-                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding, &biff)?;
+                        let (pos, sheet) = parse_sheet_metadata(&mut r, &encoding, biff)?;
                         self.metadata.sheets.push(sheet.clone());
                         sheet_names.push((pos, sheet.name)); // BoundSheet8
                     }
@@ -393,11 +393,11 @@ impl<RS: Read + Seek> Xls<RS> {
                     }
                     //0x0201 => cells.push(parse_blank(r.data)?), // 513: Blank
                     0x0203 => cells.push(parse_number(r.data, &self.formats, self.is_1904)?), // 515: Number
-                    0x0204 => cells.extend(parse_label(r.data, &encoding, &biff)?), // 516: Label [MS-XLS 2.4.148]
-                    0x0205 => cells.push(parse_bool_err(r.data)?),                  // 517: BoolErr
+                    0x0204 => cells.extend(parse_label(r.data, &encoding, biff)?), // 516: Label [MS-XLS 2.4.148]
+                    0x0205 => cells.push(parse_bool_err(r.data)?),                 // 517: BoolErr
                     0x0207 => {
                         // 519 String (formula value)
-                        let val = DataType::String(parse_string(r.data, &encoding, &biff)?);
+                        let val = DataType::String(parse_string(r.data, &encoding, biff)?);
                         cells.push(Cell::new(fmla_pos, val))
                     }
                     0x027E => cells.push(parse_rk(r.data, &self.formats, self.is_1904)?), // 638: Rk
@@ -468,6 +468,7 @@ struct Bof {
 }
 
 /// https://www.loc.gov/preservation/digital/formats/fdd/fdd000510.shtml#notes
+#[derive(Clone, Copy)]
 enum Biff {
     Biff2,
     Biff3,
@@ -510,7 +511,7 @@ fn parse_bof(r: &mut Record<'_>) -> Result<Bof, XlsError> {
 fn parse_sheet_metadata(
     r: &mut Record<'_>,
     encoding: &XlsEncoding,
-    biff: &Biff,
+    biff: Biff,
 ) -> Result<(usize, Sheet), XlsError> {
     let pos = read_u32(r.data) as usize;
     let visible = match r.data[4] & 0b0011_1111 {
@@ -683,7 +684,7 @@ fn rk_num(rk: &[u8], formats: &[CellFormat], is_1904: bool) -> DataType {
 fn parse_short_string(
     r: &mut Record<'_>,
     encoding: &XlsEncoding,
-    biff: &Biff,
+    biff: Biff,
 ) -> Result<String, XlsError> {
     if r.data.len() < 2 {
         return Err(XlsError::Len {
@@ -708,7 +709,7 @@ fn parse_short_string(
 }
 
 /// XLUnicodeString [MS-XLS 2.5.294]
-fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: &Biff) -> Result<String, XlsError> {
+fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: Biff) -> Result<String, XlsError> {
     if r.len() < 4 {
         return Err(XlsError::Len {
             typ: "string",
@@ -731,7 +732,7 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: &Biff) -> Result<String,
 fn parse_label(
     r: &[u8],
     encoding: &XlsEncoding,
-    biff: &Biff,
+    biff: Biff,
 ) -> Result<Option<Cell<DataType>>, XlsError> {
     if r.len() < 6 {
         return Err(XlsError::Len {