286630 - nsSchemaLoader doesn't bail on invalid elements as direct children of xs:schema

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[Doron Rosenberg (IBM)]

Spec: http://www.w3.org/TR/xmlschema-1/#element-schema and
http://www.w3.org/TR/xmlschema-1/#Identity-constraint

http://lxr.mozilla.org/seamonkey/source/extensions/webservices/schema/src/nsSchemaLoader.cpp#858
We only fail if rv is failure, but we never handle unknown elements in the
if/else conditions.

Comment 1

[Doron Rosenberg (IBM)]

Attachment: 1.0

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 177766 [details] [diff] [review]
1.0

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

>+    } else if ((tagName != nsSchemaAtoms::sAnnotation_atom) &&
>+               (tagName != nsSchemaAtoms::sInclude_atom) &&
>+               (tagName != nsSchemaAtoms::sImport_atom) &&
>+               (tagName != nsSchemaAtoms::sRedefine_atom) &&
>+               (tagName != nsSchemaAtoms::sNotation_atom) &&
>+               (tagName != nsSchemaAtoms::sAnnotation_atom) &&
>+               (tagName != nsSchemaAtoms::sUnique_atom) &&
>+               (tagName != nsSchemaAtoms::sKey_atom) &&
>+               (tagName != nsSchemaAtoms::sKeyref_atom) &&
>+               (tagName != nsSchemaAtoms::sSelector_atom) &&
>+               (tagName != nsSchemaAtoms::sField_atom)) {

You have sAnnotation__atom twice. I don't think the identity-constraint
elements should be in this if, the comment probably indicates that they're
ignored in ProcessElement etc. but they're not valid children of the schema
element.
And drop the inner braces, they're not needed.

>     if (NS_FAILED(rv)) {
>       nsAutoString elementName;
>       nsresult rc = aElement->GetTagName(elementName);
>       NS_ENSURE_SUCCESS(rc, rc);
> 
>       nsAutoString errorMsg;
>       errorMsg.AppendLiteral("Failure processing schema, unexpected element \"");
>       errorMsg.Append(elementName);
>       errorMsg.AppendLiteral("\" in <schema .../>");

Bah, this error message is shown for all errors, even those unrelated to
unexpected elements.

Comment 3

[Doron Rosenberg (IBM)]

Attachment: v 2

fixed it and moved error firing to the else case.

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 177880 [details] [diff] [review]
v 2

>Index: extensions/webservices/schema/src/nsSchemaAtomList.h
>===================================================================

> SCHEMA_ATOM(sPattern_atom, "pattern")
>+

No need to add this newline.

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

>+    } else if (tagName != nsSchemaAtoms::sAnnotation_atom &&

    }
    else if {

Comment 5

[Doron Rosenberg (IBM)]

checked in.