Skip to content
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

Fixes a serious problem in xmlbeans #10

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

reinerben
Copy link

Fixes a serious problem which completely messed up getters and setters in case there are an attribute and element with same name in the schema:
SchemaTypeCodePrinter.java#printStaticFields writes an array named PROPERTY_QNAME. It depends on the supplied argument Map<QName, Integer> qnameMap. But QName is not unique. If there is an attribute and element with same name, the second replaces the first when the Map is filled although the index is incremented for each of them. So, indices of all subsequent members are pointing to the wrong entry.
Now SchemaProperty is used as key for above map instead of QName. SchemaProperty instances are unique.

…s in case there are an attribute and element with same name in the schema:

SchemaTypeCodePrinter.java#printStaticFields writes an array named PROPERTY_QNAME. It depends on the supplied argument Map<QName, Integer> qnameMap. But QName is not unique. If there is an attribute and element with same name, the second replaces the first when the Map is filled although the index is incremented for each of them. So, indices of all subsequent members are pointing to the wrong entry.
Now SchemaProperty is used as key for above map instead of QName. SchemaProperty instances are unique.
@reinerben reinerben closed this Nov 28, 2023
@reinerben reinerben reopened this Nov 28, 2023
@pjfanning
Copy link
Contributor

I raised https://issues.apache.org/jira/browse/XMLBEANS-644

Could you provide a schema file that can be used for testing this?

@pjfanning
Copy link
Contributor

I ran the unit tests with this change and they pass - but that is not not enough without a new test case to validate the changes.

@reinerben
Copy link
Author

reinerben commented Nov 28, 2023 via email

@pjfanning
Copy link
Contributor

pjfanning commented Nov 28, 2023

I created my own xsd. It would be nice to get a better one.

<xs:schema attributeFormDefault="qualified" elementFormDefault="qualified"
           targetNamespace="https://xmlbeans.apache.org/XMLBEANS-644"
           xmlns:xs="http://www.w3.org/2001/XMLSchema">
  <xs:element name="xmlbeans644">
    <xs:complexType>
      <xs:sequence>
        <xs:element type="xs:string" name="data"/>
        <xs:element name="file">
          <xs:complexType>
            <xs:simpleContent>
              <xs:extension base="xs:string">
                <xs:attribute type="xs:string" name="data"/>
              </xs:extension>
            </xs:simpleContent>
          </xs:complexType>
        </xs:element>
      </xs:sequence>
    </xs:complexType>
  </xs:element>
</xs:schema>

The scomp tool fails to generate code for this schema. The code doesn't even get to the SchemaTypeCodePrinter stage. XMLBeans has code in StscState that validates the schema. There could be a different issue with my xsd but StscState rejects it in the void addDocumentType(SchemaTypeImpl type, QName name) call because of the duplicate QName.

So far, it looks like bigger changes are needed than just changing SchemaTypeCodePrinter.

There are also validations in SchemaTypeSystemImpl that appear to fail too - again because duplicate QNames will make assertContainersHelper fail.

@reinerben
Copy link
Author

reinerben commented Nov 28, 2023 via email

@reinerben
Copy link
Author

reinerben commented Nov 28, 2023 via email

@pjfanning
Copy link
Contributor

the new xsd has the same issues with validations failing in StscState and SchemaTypeSystemImpl - the scomp job does not get as far as SchemaTypeCodePrinter

@reinerben
Copy link
Author

reinerben commented Nov 28, 2023 via email

@reinerben
Copy link
Author

reinerben commented Nov 29, 2023 via email

@pjfanning
Copy link
Contributor

added with e46a709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants