From 639d17d2596ef75c2d4951d41356c25c45db02f0 Mon Sep 17 00:00:00 2001
From: Aaron Hill <aa1ronham@gmail.com>
Date: Sat, 6 Jan 2024 18:42:13 -0500
Subject: [PATCH] Apply 'xml["@myattr"]' logic when setting/deleting properties

We previously only applied this when getting properties.
---
 core/src/avm2/object/xml_object.rs            |  45 +++++++++++-------
 .../xml_attribute_name/{test.as => Test.as}   |  23 ++++++++-
 .../swfs/avm2/xml_attribute_name/output.txt   |  30 ++++++++++++
 .../swfs/avm2/xml_attribute_name/test.fla     | Bin 0 -> 3763 bytes
 .../swfs/avm2/xml_attribute_name/test.swf     | Bin 1043 -> 895 bytes
 5 files changed, 80 insertions(+), 18 deletions(-)
 rename tests/tests/swfs/avm2/xml_attribute_name/{test.as => Test.as} (57%)
 create mode 100755 tests/tests/swfs/avm2/xml_attribute_name/test.fla

diff --git a/core/src/avm2/object/xml_object.rs b/core/src/avm2/object/xml_object.rs
index 9504026f167d..0baeefb359d9 100644
--- a/core/src/avm2/object/xml_object.rs
+++ b/core/src/avm2/object/xml_object.rs
@@ -274,19 +274,7 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
             }
         }
 
-        // Special case to handle code like: xml["@attr"]
-        // FIXME: Figure out the exact semantics.
-        let multiname = if !name.has_explicit_namespace()
-            && !name.is_attribute()
-            && !name.is_any_name()
-            && !name.is_any_namespace()
-        {
-            name.local_name()
-                .map(|name| string_to_multiname(activation, name))
-        } else {
-            None
-        };
-        let name = multiname.as_ref().unwrap_or(name);
+        let name = handle_input_multiname(name.clone(), activation);
 
         let matched_children = if let E4XNodeKind::Element {
             children,
@@ -302,7 +290,7 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
             search_children
                 .iter()
                 .filter_map(|child| {
-                    if child.matches_name(name) {
+                    if child.matches_name(&name) {
                         Some(E4XOrXml::E4X(*child))
                     } else {
                         None
@@ -392,6 +380,8 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
         value: Value<'gc>,
         activation: &mut Activation<'_, 'gc>,
     ) -> Result<(), Error<'gc>> {
+        let name = handle_input_multiname(name.clone(), activation);
+
         // 1. If ToString(ToUint32(P)) == P, throw a TypeError exception
         if let Some(local_name) = name.local_name() {
             if local_name.parse::<usize>().is_ok() {
@@ -472,7 +462,7 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
             };
 
             let mc = activation.context.gc_context;
-            self.delete_property_local(activation, name)?;
+            self.delete_property_local(activation, &name)?;
             let Some(local_name) = name.local_name() else {
                 return Err(format!("Cannot set attribute {:?} without a local name", name).into());
             };
@@ -507,7 +497,7 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
 
         // 9. Let i = undefined
         // 11.
-        let index = self_node.remove_matching_children(activation.gc(), name);
+        let index = self_node.remove_matching_children(activation.gc(), &name);
 
         let index = if let Some((index, node)) = index {
             self_node.insert_at(activation.gc(), index, node);
@@ -606,6 +596,8 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
         activation: &mut Activation<'_, 'gc>,
         name: &Multiname<'gc>,
     ) -> Result<bool, Error<'gc>> {
+        let name = handle_input_multiname(name.clone(), activation);
+
         if name.has_explicit_namespace() {
             return Err(format!(
                 "Can not set property {:?} with an explicit namespace yet",
@@ -627,7 +619,7 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
         };
 
         let retain_non_matching = |node: &E4XNode<'gc>| {
-            if node.matches_name(name) {
+            if node.matches_name(&name) {
                 node.set_parent(None, mc);
                 false
             } else {
@@ -643,3 +635,22 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
         Ok(true)
     }
 }
+
+fn handle_input_multiname<'gc>(
+    name: Multiname<'gc>,
+    activation: &mut Activation<'_, 'gc>,
+) -> Multiname<'gc> {
+    // Special case to handle code like: xml["@attr"]
+    // FIXME: Figure out the exact semantics.
+    if !name.has_explicit_namespace()
+        && !name.is_attribute()
+        && !name.is_any_name()
+        && !name.is_any_namespace()
+    {
+        name.local_name()
+            .map(|name| string_to_multiname(activation, name))
+            .unwrap_or(name)
+    } else {
+        name
+    }
+}
diff --git a/tests/tests/swfs/avm2/xml_attribute_name/test.as b/tests/tests/swfs/avm2/xml_attribute_name/Test.as
similarity index 57%
rename from tests/tests/swfs/avm2/xml_attribute_name/test.as
rename to tests/tests/swfs/avm2/xml_attribute_name/Test.as
index e76a1166f592..e55385fb7292 100644
--- a/tests/tests/swfs/avm2/xml_attribute_name/test.as
+++ b/tests/tests/swfs/avm2/xml_attribute_name/Test.as
@@ -1,4 +1,4 @@
-package {
+package {
     import flash.display.Sprite;
 
     public class Test extends Sprite {
@@ -22,6 +22,27 @@ package {
             trace("xml[name2]: " + xml[name2]);
             trace('xml["bar"]: ' + xml["bar"]);
             trace('xml["@bar"]: ' + xml["@bar"]);
+
+            // Test creating attribute
+            xml["@foo"] = "My new attr";
+            trace('xml["@foo"]: ' + xml["@foo"]);
+            trace(xml.toXMLString());
+
+            trace("Has property: " + xml.hasOwnProperty("@foo"));
+
+            delete xml["@foo"];
+            trace('After delete: xml["@foo"] = ' + xml["@foo"]);
+            trace(xml.toXMLString());
+
+            var list = new XMLList(xml + xml);
+            trace("List: " + list);
+            trace('list["@bar"] = ' + list["@bar"]);
+            trace(list);
+            delete list["@bar"];
+            trace("After delete:");
+            trace('list["@bar"] = ' + list["@bar"]);
+            trace(list);
+
         }
     }
 }
diff --git a/tests/tests/swfs/avm2/xml_attribute_name/output.txt b/tests/tests/swfs/avm2/xml_attribute_name/output.txt
index 64618c70b5a6..1acc462b250e 100644
--- a/tests/tests/swfs/avm2/xml_attribute_name/output.txt
+++ b/tests/tests/swfs/avm2/xml_attribute_name/output.txt
@@ -8,3 +8,33 @@ xml[name2]: element
 xml[name2]: new element
 xml["bar"]: new element
 xml["@bar"]: new attribute
+xml["@foo"]: My new attr
+<foo bar="new attribute" foo="My new attr">
+  <bar>new element</bar>
+</foo>
+Has property: true
+After delete: xml["@foo"] = 
+<foo bar="new attribute">
+  <bar>new element</bar>
+</foo>
+List: <foo bar="new attribute">
+  <bar>new element</bar>
+</foo>
+<foo bar="new attribute">
+  <bar>new element</bar>
+</foo>
+list["@bar"] = new attributenew attribute
+<foo bar="new attribute">
+  <bar>new element</bar>
+</foo>
+<foo bar="new attribute">
+  <bar>new element</bar>
+</foo>
+After delete:
+list["@bar"] = 
+<foo>
+  <bar>new element</bar>
+</foo>
+<foo>
+  <bar>new element</bar>
+</foo>
diff --git a/tests/tests/swfs/avm2/xml_attribute_name/test.fla b/tests/tests/swfs/avm2/xml_attribute_name/test.fla
new file mode 100755
index 0000000000000000000000000000000000000000..55fbf1897c5f15d87ab8fa435fb86664f2b14400
GIT binary patch
literal 3763
zcmbVPc{G&m8=kRbOh%R=gh(kw82i%D$dV-nWv?+N)Yzx&jKNqEiYSzd>}$5{OUM@4
zB7AmPLm_18H}m<*SEs%|e)oIs_j%9r-q&;AGv|8Gxh`EzN-8J-00IC6!uXBK?~M`J
z0RR9wNm~FrTRRNS-4TOwbi~@CQ8-%%`!l!ftwc~(4we`Zw`*A4pWs{&D@s}tjGSaR
z4&#FRy9#;H$XFPmFZpvhsp3bF%e6GlA(fHFXZ~D8K3My_fwHiMj_QAuF_Ow0Z(3q)
zU2Jd|J4Y;vG{`^Y6d=&wzZAsYw+yBL0F0>sfJ3Bmm5bUc4(OY97<<yhcGy4z^N0aa
z2Cv+nZ&yn@Mj7fDbtaRaM6%L)sISgvJKux6yt&YHD|~rr{y3G>c=*@mPm2p-)_o7v
zI^xw+41sk|%0|z@-^EdHT-DlCZ~ElM$^9CU{-yqA0^??p@lqEh6ju`<Wsr=Q@1f>h
zjl3rAhY~!9oUW+dq)IA8wm5sCsU7)wYWQxqZ64B%(_p;X$0e7et*>;)K<I8OAV`}R
z3cjR^4!NMCEn>(xny6*<IZe!CcGUc9c$E_Io_>VVv_wyjsz!qwai|lTMA*GM7s~Eh
zn`?L%|AIz496zvtWXNmD!p0Oxb%n|(#x@>XduQK=i~+U_^f@vQb}7!R6Y`%?2iR_T
zH#f7>xCoA|tQ$f{PdPuc$q%N9Y7pmkiwcx~(2yeydL2HkX1^io9W37_JK`D(n=fA4
zu$b}4U(!CAFTFC+|H*oylM3#=jgOO*o)($eO_nb%c)LzRE7V~!AcMzdW?AYoMt{hI
zN{mHP_osoEGbzmPOBs$F5aFEXAJclf=xa(aaJJ>zos`Kpk9C*r=1Fm_;gxg^&Q<^1
zIJADeg0=e#@V&C1Tn)Xd?^xm8hlWz|WxM7vnR1s*R`3#=@DsDLOBVQ{N@dgJn~RXG
z<|Df$_^P5<0Ul|zXkWVJRIlO((}n<G5heOT{}t*M!7;6)Ribu@^Xak<8ee^zyWx9e
zr1<9pAu;7gu#ld9kdgGApsY|h6q!8VA$v^wlzZVJ8J@!l05Fpvx_iXb$KY_b_SP<B
zK!dS^1BWcLuWus7)9xhsS#={;3TRo^fphoNc&EUXLkE+ajOkt)zw@-9)PV26c1yA#
z(JI-G^`DEcthhD8xI6me7Zp_Be-^{wbSx97t8v0l3TsdENo*Q@Y0sEK5-x3^OCzRj
zxK^A6OkhOj-Tb!q9I^gaE<9hf*?~r3WuIUJH-bB%FdfHBb+jI#pR#zsb+G3%+=bWt
z`V>ptP8_?;*9d*8WHfY{)2W49oynklnsq6dPR}X?WD#<DQ;;=oDwn`&B@)mOK~a_B
z#>=poDS48K5e{xD?jmgH0nQ5ngA)nNJC9sS;ped<lGcs+PrZX8v~k2+C`bO6y4;*<
z?aH@!T_(*=9ZD{LJBeqj?`3p}RG0~W;14af1UwWL3y1l{*iC!LJA3(_SrFKu9^7)m
z@~aFz4(aL{X0T2HRVpHQu4{7Y1y>2v;)j^kbW}!Ix8p>5QeL*|a4G{Pb7Y+Jfc7B1
zQsHpU1-vi~e&}$YeZEaJ$bk0S+%dWx)$;KmpIoFvZ`}RXYenX^CGl@seX5Us@rcE^
z)7{pO$SAK^aQF9Ed*<B9)+4wUM-#Vwy;2-BpKVc~=WwNWe#x+pX-*_SZX|Y21U$#_
zaXmc~G9X(0o?|0V?s?ABo}#MM98aI*Vb&X;Aj$3N7q(*sSG?PJ&-lH36m&$zl2?Gv
z*P$Qj7~f_)+P9k)>dRG~5i$_S%RyX*jIq~&JnayLbe$3bofEwf)T&!!+VoDRlu9du
zHo}bU87jDp*9*}@Hzx4xIx$0ZQqt=@0nD%fGPQt2;{wDqKxj)pE>spR1?dKy;6iBy
zr1D9CmHH_VMd4uozGi=6L^@&X1fM%*p)&x{!E*9cXW*jet1WQd19S{ectm>tVciR6
zz2hoyRdA2tT;7^@ptGQO-ea$}9J9exJIgz_vlJAb@R#QG^eMtYne2zG8e^4{fT1ku
zqerw@HMx5-@izwC&a;00@)T70E?`+JN1F*;ktD&Q98(SEaH-z_Wy2JhyUt%asG=^L
z4yt5rX_5wsg+7175@0f;NHv{$FRF=ggy#No;teSCE=N!KG|_<4dmGw4^RVeGpq9CH
z;FT6b6#HNFvsyWoHMZKvHPNeb_01+($xR&kN0`QdGD%^R2^^0{S+sObPj0C#y;{{g
zFpViwK1gIb3+XP5kck`CTXy7VpJ9GXr1cvQ)-1Z7E!EIG5&H?sbW?}*22n*aeprj>
zs{d$5j(z>~Y<o%aAgzf4PYhi_U`1HJdj6@wz`io>+k=hiaSBgu6YZIsOb`NIN^JF$
z)Q$*&ZJ3Tglv8&!p*AdSfI>R?>@k6pLZ2ZLdCM2*2Jx&WjAv1&>Ykq77B5*BmG?F2
zJ*xgh%$jYHCgauSxD|BcnR-MHk_dB?mbha;FHo9!=hXSqlw+n0d8_@w;Q$3cwufV4
z<|koO)pv{_qG?<1)Y8{1tZGBlBIia^2Rt>lieS3aVe@mx5c1({8l`$PaffO>;`LU;
zSU2X*V|VbgWj=tSdbQv)1w66(Sr)bZC2wJQi%O+W<aCyO8~uk<w`xDG1s(D)cl26e
z^LaSr_0`MMcjpuQTb965Fx6!PwK<{owkS-ck&;E2sK3fY#yM^!bEjyGY0EWbz5c5y
z{^^gnx*&JQ@`0wHbMo%m7%msy6K?TXb=A?K;bw2joAk)jw&Is>NowaecoD_ld2sWk
z$!)-TxO!RWAyDV_ngsusvq>A=E}+*1*B{HK_Gu&)%^Xl{>Nx#CDTvr$VpR24o+t$-
zncr0x7HFo_iIljT&>mDLnd;5i5R9@(`*=`5pUS2AtX~d|iC^?NVh44Cf@`E%TeO65
zRKW+UPyzIs&xlwG<!yx2hxwV(h%uW376=@gMk1Y|VV-^0QwC`p=~CvvCpAd$MM1~d
zGE?}pxa(Jn5KoP)l>f@ikP&(}8NFcWIvK+I#Y2tBiJvm2;aD|2WR_m6>WCPpzQikR
z@aDaCf-qv4M^Q8nj@}jp`RqVAvg8V?O&iiC6Ae31!1tw0J}NI}>t1?XDGyzJqn?d4
zU=9SZ3T54+F!CEh=U!8<RG6v>3B-qMomIcTZ10mbkza=eUfg)e#@$&O!5-8aUv5y~
zW=@2uz4gnr3QDYz%FTN#$kguF#-g?o=usSD>;T^q*Q>DJK0LgUn!~<SxlX7GJWV{3
zwFH&TjB>#p>4~?<>?GE+gq^)OJ`|EsEbsf;feulh^gOG@`vsGA%Yo3v!LzKD)%+`o
zA1kUZv|M=H?O^_SEEUh=`IdeXV)uIHN%pC!&s(V}VcD_VcLZ7qCP}=f*^AlOT6ZkX
zRM-39@*)n-yTP#7fORJ@HrvAeSViFpFJtfoE~IRFvtNF@cu45$4sEti@rdFYwb?DM
zJfS5hy%iNq=Q82&dPqX2+T5;51<cA0C=E`?p<OO)M%q9%jtd1f95ydeC@l7xUXO?8
z&gSmuW^3}^j&ZDMJ<&2PcQt4t^<%|c=~sUrSqAYJmzp|X)w%3&pUHFx98tgC(QmhG
zYUb5_5<W40rSJJ5iv+<{o~?6r&2G3-2<o;LU_Es;e0-r2)hcyc9~SH=$-HSu5A}8?
zmURwAzNb6+**U)Q4P%)n@{9fOOyq8qzHHiKcX$FYwW{lSJOX-O^+V?NH_10sN2z?8
zn!88Bi>7GqZENbZRL3UZkl$Duw?8@^Y9f3CewPe;GW>_cAU!aVRfdD5Ef(`{#qhI4
z9_)JfjifFB<RnWBOI!Oh`tEir7)K0AtfNtA8;mt+dxi!GI6%6X0K9)Ie9{)6Dx0K8
zeiHHjhX(*sKmq$E;#U_yZnGC~((=O-X#R})UPE5VW?(NENQ*rw_pQO7QDk$l7i3g4
zDSxOl-T!DtHVb<}hLw}@H`ZaV>z~ZSKX}qIN6KHU#P{BRl92mgUdn%s{Ij6^x&N<{
za{mYe(h<KAnEy2cnPB@7bny3d`}uGGm2ms`5)%G5^!q-*pN!`I0aSGRwo_M=`T+S2
Pm~=&w%o>994gmNc+e6Zs

literal 0
HcmV?d00001

diff --git a/tests/tests/swfs/avm2/xml_attribute_name/test.swf b/tests/tests/swfs/avm2/xml_attribute_name/test.swf
index f098ba34faa0a4e165ac43e806d1a50d6d96a264..f6ddd8107eba8560717c7204387808c654d057f3 100644
GIT binary patch
literal 895
zcmV-_1AzQPS5qsf1pokeoPAX5Qrkup-qqEzeBo*w14VI60zr`knf8ad6o!<BOfs}X
zJCjNAVA-r{6I=F3YZLzRzwf}C<P{K1+s8nC0(w@KNt|@5v3Jk+oo~P0J!hm3NIXU;
zdx=mUqZi`|gwQT`cXzii)_YJ|E$z6aPXDxT_PRgs_0RWDTB`@Ye!Khby@5VXtDwc`
zuufc}ts?YsYExtcJY&dUlZBLDh)+#62y93@E!%GxCr!WGvd^W%uGgew{ayLU>p4z~
z98u8t@#vp9{*m2puN{48w~lQ}y=JXP$+2s<$@)5TGi;xSoWK~74QoQH=~Mpgo7c}a
zoK8op+1_UHer0iM1BflsB5mT*4U>Tl6LecVgi7pKvK}@l?Uqf`KIznmQSY?Pox|tm
znq_@w)_TnrZMst<*C-FJQ5q#*4CaH{DXG)lj!W&POT2|q(Y{NGXV+=7PX@x-|1p^x
z<VYV*@0WjI6U*DQ-XNYx!H48n6|_was$y)zr3{A4nhN+g7FNny+9hY&eN9p<Qv_v^
zr6RB$BY_3^sI0N$&~RzFtfhvQYamsbXIxid8!D>4IoF2!#IC?HSCw|4J%_Np`+Mf4
z?Q31H(<L4~FKd~G?H`=EKgGn{jswNfPQaJK9X@k`<H%t08hS0Up{!+D--kzsbb4g1
zG^k(d^ZI4Ks9$=d)Z<?@Cima{%;s=T$nzpEW@d0ATi^?0h4I2fVX`olTj5dy7A2Wa
zC`mPy9+xpMbE1sZOd2Z*EaT}}tUy@AN(s<o89$XXy39S3IYZ`5e()}F(MyIf<`h06
zFo&^#5dfS}_$%v2pafA#{=xqvh2oV3|9y0eWhC9=1Xgab!cE-bB)=F+6<$xh;_=0w
zp%OU4o`^2~3X>H{&s|?0NH|m>oC&TSIYO!6TF={Zm>yv>fF1;TI)+(*g+QN;;Y8ph
znC;);BqZi;B614C`QRp&Ob0jmH(;9^VP{5d^P{%2fxe(?uxJkWhX7)Jz9I#C3S!{`
zWFA3gFNvcRQ=Q7oVG;PnPb(gMTCrGBgW&R&iZALy_!wkQYC@!W0`XE!xZi!LBGm*I
zEHjdsJ_W@xV^+S9>O-jkUE!cniIpo0+9D8CO<C2nRn1t{tW_PesvP{@qWS1)V9;=|
V<oiiw&~C)OMra#?e*q^4qH7$#xn=+W

literal 1043
zcmV+u1nm1mS5qdR1^@tf0fki0Z{tK1p0S-c&aeL5v`xFLx7(##l*Dcdi<(WNtu_?x
z3MkruSb?Hw;*1+p$F}TA(>)>WnLmOPLgJ6$#DQDW3OI0Cae-UIUr^pSNt-MOEZLs-
zzVCg0@68*ZBI!?r(%&Prj8S!N1tIj!!e1DnvfnaG&#MROX~%Yg5?FhiZN|KkruBNg
zLT{(w`bXOK?(VLp7qwzB4~%^9(qZH@?*#WYE1^J@28M5W%yJ!-3zDYWWqX^OBh{91
zE$VfBJCtl08ntPMIxNt(3)>*nGD@cFcL=Kx&$BIqa5?QXAGBTLxJOQC-n2>3E^9YL
z9>XkV)5^X_j5bvdY<jBh--L(cA%k;UH+fb@Rhke{AYpWvR~RwL<8fZMX<I>?`s44^
zT$A;P4<B^9PAkkb47?%oFvF*Wx&Iq9rtuJ)I7eM_L@U&3d|nGRuU%mUgwe_~;;8TC
z)S_N|cbG1BmNnk_iJcgEU~?-dJ=<OR<S_r~ey#S!;qxjwn}G?Kx~j8(i)HjgxVpOf
zGA;rXg<G0E{$&TI1fd__`QeXmVfAzTGa4}T(_&FVh$Epniwc~yC54u%wcQTYnA_1H
zonz*DTDWtM9&xxp0uhEya!7|p_@gMk`1qrI*>qjCN&LNa!kBM0yNs?^%HXJs#-yxq
zqpSh1BEm9E1jIIqyLU%>Nm_1`77Y0Aer-o9>iR>i*|ltDIg4Qeg_ad~HhEd9dA`Ny
z#<a6K1P_~EQ-eKmVZ<$m`gf*DPaQ^mVleB34vDMN_>{=n5Thr2%1a;c7sm;MQS~DX
zbZCZmK;7d9rSKT4Dsa1`CcyrRFY6^W<<Opb-IOFp)`7Xsq0r!Q+>-cWIc(`jaV+>?
ztE7g3Aqdn2*Bmug;k%;oM{25;=&(!CVVyH?q@SFuEj3m4+<%&snZmDoyVhtNxeewv
z3^*l?7Bwx$;)lM`rnV<~ZXj;!?=Ou>Z{Whi?U@M92$A@b5KAoMEY2pgscbqslby}1
zWmF-F1yPDb<(LvrB-64WrxmP3uq@22Vnu;BmQ%N7;Vn6RM^4?9Q|of-9t^$!gS-m2
zfP@Wv|LskD{{aS#XgKW{f~puWEFz3!ig>1f2<JeOl=y4>k2r{&>2N<qmsmp4OB}(<
zC5{U7gJ>P+V$U#s_3J?Co5F#JUi~(R*QMP2`F95r-Wnuez0f~5BSSOUKhG_a$RIWC
zPJ?-?pHs%P8L%_`+|t;d@0(Gs{rg}6kmZY^*dnY~`WIu;t^P$W3)-1!_Y#<s+RI>1
zYOg?{f&fMDp6O7jL{VKa6}=ue<9a<|CiHsJOzQQNnbPYqGZsExpxfcqmLWm_0cGL(
NpYa%a{4eqj{!fy44S@gv