Skip to content

avm2: Limited XML namespace support #12540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 102 additions & 25 deletions core/src/avm2/e4x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use std::{
use gc_arena::{Collect, GcCell, MutationContext};
use quick_xml::{
events::{BytesStart, Event},
Reader,
name::ResolveResult,
NsReader,
};

use crate::{avm2::TObject, xml::custom_unescape};
Expand All @@ -26,6 +27,7 @@ pub struct E4XNode<'gc>(GcCell<'gc, E4XNodeData<'gc>>);
#[collect(no_drop)]
pub struct E4XNodeData<'gc> {
parent: Option<E4XNode<'gc>>,
namespace: Option<AvmString<'gc>>,
local_name: Option<AvmString<'gc>>,
kind: E4XNodeKind<'gc>,
}
Expand Down Expand Up @@ -72,6 +74,7 @@ impl<'gc> E4XNode<'gc> {
mc,
E4XNodeData {
parent: None,
namespace: None,
local_name: None,
kind: E4XNodeKind::Element {
attributes: vec![],
Expand All @@ -86,17 +89,24 @@ impl<'gc> E4XNode<'gc> {
mc,
E4XNodeData {
parent,
namespace: None,
local_name: None,
kind: E4XNodeKind::Text(text),
},
))
}

pub fn element(mc: MutationContext<'gc, '_>, name: AvmString<'gc>, parent: Self) -> Self {
pub fn element(
mc: MutationContext<'gc, '_>,
namespace: Option<AvmString<'gc>>,
name: AvmString<'gc>,
parent: Self,
) -> Self {
E4XNode(GcCell::new(
mc,
E4XNodeData {
parent: Some(parent),
namespace,
local_name: Some(name),
kind: E4XNodeKind::Element {
attributes: vec![],
Expand All @@ -116,6 +126,7 @@ impl<'gc> E4XNode<'gc> {
mc,
E4XNodeData {
parent: Some(parent),
namespace: None,
local_name: Some(name),
kind: E4XNodeKind::Attribute(value),
},
Expand Down Expand Up @@ -195,6 +206,7 @@ impl<'gc> E4XNode<'gc> {
mc,
E4XNodeData {
parent: None,
namespace: this.namespace,
local_name: this.local_name,
kind,
},
Expand Down Expand Up @@ -305,7 +317,7 @@ impl<'gc> E4XNode<'gc> {
};

let data_utf8 = string.to_utf8_lossy();
let mut parser = Reader::from_str(&data_utf8);
let mut parser = NsReader::from_str(&data_utf8);
let mut open_tags: Vec<E4XNode<'gc>> = vec![];

let mut top_level = vec![];
Expand Down Expand Up @@ -373,6 +385,7 @@ impl<'gc> E4XNode<'gc> {
activation.context.gc_context,
E4XNodeData {
parent: None,
namespace: None,
local_name: None,
kind: if is_text {
E4XNodeKind::Text(text)
Expand All @@ -393,17 +406,17 @@ impl<'gc> E4XNode<'gc> {

match &event {
Event::Start(bs) => {
let child = E4XNode::from_start_event(activation, bs, parser.decoder())
.map_err(|_| malformed_element(activation))?;
let child =
E4XNode::from_start_event(activation, &parser, bs, parser.decoder())?;

if let Some(current_tag) = open_tags.last_mut() {
current_tag.append_child(activation.context.gc_context, child)?;
}
open_tags.push(child);
}
Event::Empty(bs) => {
let node = E4XNode::from_start_event(activation, bs, parser.decoder())
.map_err(|_| malformed_element(activation))?;
let node =
E4XNode::from_start_event(activation, &parser, bs, parser.decoder())?;
push_childless_node(node, &mut open_tags, &mut top_level, activation)?;
}
Event::End(_) => {
Expand Down Expand Up @@ -448,6 +461,7 @@ impl<'gc> E4XNode<'gc> {
activation.context.gc_context,
E4XNodeData {
parent: None,
namespace: None,
local_name: None,
kind: E4XNodeKind::Comment(text),
},
Expand Down Expand Up @@ -485,6 +499,7 @@ impl<'gc> E4XNode<'gc> {
activation.context.gc_context,
E4XNodeData {
parent: None,
namespace: None,
local_name: Some(name),
kind: E4XNodeKind::ProcessingInstruction(value),
},
Expand All @@ -506,36 +521,85 @@ impl<'gc> E4XNode<'gc> {
/// valid encoded UTF-8 data. (Other encoding support is planned later.)
pub fn from_start_event(
activation: &mut Activation<'_, 'gc>,
parser: &NsReader<&[u8]>,
bs: &BytesStart<'_>,
decoder: quick_xml::Decoder,
) -> Result<Self, quick_xml::Error> {
// FIXME - handle namespace
let name =
AvmString::new_utf8_bytes(activation.context.gc_context, bs.local_name().into_inner());

) -> Result<Self, Error<'gc>> {
let mut attribute_nodes = Vec::new();

let attributes: Result<Vec<_>, _> = bs.attributes().collect();
for attribute in attributes? {
let key = AvmString::new_utf8_bytes(
activation.context.gc_context,
attribute.key.into_inner(),
);
let value_str = custom_unescape(&attribute.value, decoder)?;
for attribute in attributes.map_err(|_| malformed_element(activation))? {
let (ns, local_name) = parser.resolve_attribute(attribute.key);
let name =
AvmString::new_utf8_bytes(activation.context.gc_context, local_name.into_inner());
let namespace = match ns {
ResolveResult::Bound(ns) => Some(AvmString::new_utf8_bytes(
activation.context.gc_context,
ns.into_inner(),
)),
ResolveResult::Unknown(ns) if ns == b"xmlns" => continue,
// https://www.w3.org/TR/xml-names/#xmlReserved
// The prefix xml is by definition bound to the namespace name http://www.w3.org/XML/1998/namespace.
ResolveResult::Unknown(ns) if ns == b"xml" => {
Some("http://www.w3.org/XML/1998/namespace".into())
}
ResolveResult::Unknown(ns) => {
return Err(Error::AvmError(type_error(
activation,
&format!(
"Error #1083: The prefix \"{}\" for element \"{}\" is not bound.",
String::from_utf8_lossy(&ns),
name
),
1083,
)?))
}
ResolveResult::Unbound => None,
};

let value_str = custom_unescape(&attribute.value, decoder)
.map_err(|_| malformed_element(activation))?;
let value =
AvmString::new_utf8_bytes(activation.context.gc_context, value_str.as_bytes());

let attribute_data = E4XNodeData {
parent: None,
local_name: Some(key),
namespace,
local_name: Some(name),
kind: E4XNodeKind::Attribute(value),
};
let attribute = E4XNode(GcCell::new(activation.context.gc_context, attribute_data));
attribute_nodes.push(attribute);
}

let (ns, local_name) = parser.resolve_element(bs.name());
let name =
AvmString::new_utf8_bytes(activation.context.gc_context, local_name.into_inner());
let namespace = match ns {
ResolveResult::Bound(ns) => Some(AvmString::new_utf8_bytes(
activation.context.gc_context,
ns.into_inner(),
)),
ResolveResult::Unknown(ns) if ns == b"xml" => {
Some("http://www.w3.org/XML/1998/namespace".into())
}
ResolveResult::Unknown(ns) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have the xml and xmlns cases as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if <xmlns:dummy/> is even valid XML, because xmlns: is used as an attribute usually, but when correctly binding the namespace like this it already works: <x xmlns:xmlns="http://example.org"><xmlns:a></xmlns:a></x>.

Interestingly Flash might have a bug related to xml:. <xml:a /> works, but <xml:a></xml:a> throws

The element type "a" must be terminated by the matching end-tag "</a>".

<xml:a></a> works! 🤯

Copy link
Collaborator Author

@evilpie evilpie Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I added default support for xml, but not xmlns, prefixes for elements. (Matching Flash)

<xml:a />
no error

<xmlns:a />
TypeError: Error #1083: The prefix "xmlns" for element "a" is not bound.

<root xmlns:xmlns="http://example.org"><xmlns:a /></root>
no error

return Err(Error::AvmError(type_error(
activation,
&format!(
"Error #1083: The prefix \"{}\" for element \"{}\" is not bound.",
String::from_utf8_lossy(&ns),
name
),
1083,
)?))
}
ResolveResult::Unbound => None,
};

let data = E4XNodeData {
parent: None,
namespace,
local_name: Some(name),
kind: E4XNodeKind::Element {
attributes: attribute_nodes,
Expand All @@ -555,6 +619,10 @@ impl<'gc> E4XNode<'gc> {
Ok(result)
}

pub fn namespace(&self) -> Option<AvmString<'gc>> {
self.0.read().namespace
}

pub fn local_name(&self) -> Option<AvmString<'gc>> {
self.0.read().local_name
}
Expand All @@ -573,16 +641,25 @@ impl<'gc> E4XNode<'gc> {
return false;
}

// FIXME - we need to handle namespaces here
if name.is_any_name() {
if !name.is_any_name() && self.local_name() != name.local_name() {
return false;
}

if name.is_any_namespace() {
return true;
}

if let Some(local_name) = self.local_name() {
Some(local_name) == name.local_name()
} else {
false
if name.has_explicit_namespace() {
return self.namespace() == name.explict_namespace();
}

// By default `xml.*` matches in all namespaces, unless an explicit
// namespace is given (`xml.ns::*`).
// However normal properties like "xml.prop" match only in the
// default namespace.
// TODO: Implement this by better handling default namespaces.
// See also "The QName Constructor Called as a Function".
name.is_any_name() || self.namespace().is_none()
}

pub fn descendants(&self, name: &Multiname<'gc>, out: &mut Vec<E4XOrXml<'gc>>) {
Expand Down
48 changes: 28 additions & 20 deletions core/src/avm2/globals/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,29 +334,37 @@ pub fn append_child<'gc>(
}
} else {
// Appending a non-XML/XMLList object
let last_child_name = if let E4XNodeKind::Element { children, .. } = &*xml.node().kind() {
let num_children = children.len();

match num_children {
0 => None,
_ => children[num_children - 1].local_name(),
}
} else {
// FIXME - figure out exactly when appending is allowed in FP,
// and throw the proper AVM error.
return Err(Error::RustError(
format!(
"Cannot append child {child:?} to node {:?}",
xml.node().kind()
)
.into(),
));
};
let (last_child_namespace, last_child_name) =
if let E4XNodeKind::Element { children, .. } = &*xml.node().kind() {
let num_children = children.len();

match num_children {
0 => (None, None),
_ => (
children[num_children - 1].namespace(),
children[num_children - 1].local_name(),
),
}
} else {
// FIXME - figure out exactly when appending is allowed in FP,
// and throw the proper AVM error.
return Err(Error::RustError(
format!(
"Cannot append child {child:?} to node {:?}",
xml.node().kind()
)
.into(),
));
};

let text = child.coerce_to_string(activation)?;
if let Some(last_child_name) = last_child_name {
let element_node =
E4XNode::element(activation.context.gc_context, last_child_name, *xml.node()); // Creating an element requires passing a parent node, unlike creating a text node
let element_node = E4XNode::element(
activation.context.gc_context,
last_child_namespace,
last_child_name,
*xml.node(),
); // Creating an element requires passing a parent node, unlike creating a text node

let text_node = E4XNode::text(activation.context.gc_context, text, None);

Expand Down
7 changes: 7 additions & 0 deletions core/src/avm2/multiname.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,13 @@ impl<'gc> Multiname<'gc> {
}
}

pub fn explict_namespace(&self) -> Option<AvmString<'gc>> {
match self.ns {
NamespaceSet::Single(ns) if ns.is_namespace() && !ns.is_public() => Some(ns.as_uri()),
_ => None,
}
}

/// Indicates if this multiname matches any type.
pub fn is_any_name(&self) -> bool {
self.name.is_none()
Expand Down
50 changes: 23 additions & 27 deletions core/src/avm2/object/xml_list_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,35 +275,31 @@ impl<'gc> TObject<'gc> for XmlListObject<'gc> {
}
}
}

let matched_children = write
.children
.iter_mut()
.flat_map(|child| {
let child_prop = child
.get_or_create_xml(activation)
.get_property_local(name, activation)
.unwrap();
if let Some(prop_xml) =
child_prop.as_object().and_then(|obj| obj.as_xml_object())
{
vec![E4XOrXml::Xml(prop_xml)]
} else if let Some(prop_xml_list) = child_prop
.as_object()
.and_then(|obj| obj.as_xml_list_object())
{
// Flatten children
prop_xml_list.children().clone()
} else {
vec![]
}
})
.collect();

return Ok(XmlListObject::new(activation, matched_children, Some(self.into())).into());
}

write.base.get_property_local(name, activation)
let matched_children = write
.children
.iter_mut()
.flat_map(|child| {
let child_prop = child
.get_or_create_xml(activation)
.get_property_local(name, activation)
.unwrap();
if let Some(prop_xml) = child_prop.as_object().and_then(|obj| obj.as_xml_object()) {
vec![E4XOrXml::Xml(prop_xml)]
} else if let Some(prop_xml_list) = child_prop
.as_object()
.and_then(|obj| obj.as_xml_list_object())
{
// Flatten children
prop_xml_list.children().clone()
} else {
vec![]
}
})
.collect();

Ok(XmlListObject::new(activation, matched_children, Some(self.into())).into())
}

fn call_property_local(
Expand Down
Loading