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

JSON->XML conversion adds nm:ERROR when using json instead of file input #1849

Closed
laurelmay opened this issue Jul 11, 2023 · 9 comments · Fixed by #1857 or #1856
Closed

JSON->XML conversion adds nm:ERROR when using json instead of file input #1849

laurelmay opened this issue Jul 11, 2023 · 9 comments · Fixed by #1857 or #1856
Labels

Comments

@laurelmay
Copy link
Contributor

Describe the bug

When following the steps described at "Alternate Invocations" for performing a JSON->XML conversion using the json runtime parameter results in an invalid OSCAL XML file. There is an additional nm:ERROR node at the root. This does not occur if using the file runtime parameter.

For example:

$ java -jar ~/Downloads/SaxonHE12-3J/saxon-he-12.3.jar -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json json='{ "catalog": {} }' -o:catalog.xml
$ cat catalog.xml
<?xml version="1.0" encoding="UTF-8"?>
<nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No file found at </nm:ERROR>
<catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0"/>

This can be reproduced using the xslt3 CLI from NPM as well as programming language libraries.

Who is the bug affecting

Any users who wish to use the XSLT3 files to perform document conversions without having to create an intermediate file with the JSON content.

What is affected by this bug

Tooling & API

How do we replicate this issue

  1. Invoke a Saxon CLI tool and pass the json runtime parameter, using for example -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json json='{ "catalog": {} }' -o:catalog.xml as arguments
  2. View the contents of the output file (catalog.xml)
  3. See the nm:ERROR node as the second line in the file.

Expected behavior (i.e. solution)

The <nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No file found at </nm:ERROR> value should not be emitted in the output.

Other comments

I am not an expert in XSLT3; however, I am guessing the issue is based on the fact that there is not a condition to handle whether $json has been supplied in this block that emits the error:

https://github.com/usnistgov/OSCAL/blob/d942435d493578e06507f93cf1b27afd71d2fdf5/xml/convert/oscal_complete_json-to-xml-converter.xsl#L20-L22

This error occurs for all models; however, the complete schema and catalog model are used as a consistent example for simplicity.

Revisions

No response

@laurelmay laurelmay added the bug label Jul 11, 2023
@wendellpiez
Copy link
Contributor

wendellpiez commented Jul 11, 2023

When duplicating this, I suggest we also try:

$ java -jar ~/Downloads/SaxonHE12-3J/saxon-he-12.3.jar -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json json='{ "catalog": { 'uuid': '1234' } }' -o:catalog.xml

Reason: to confirm whether the input is being correctly processed, apart from the bogus error.

Meanwhile @kylelaker as a temporary workaround while we repair this, would you be interested in a short XSLT that would scrub the false error message from your OSCAL, presumably leaving valid OSCAL (or invalid, as in the case of the example I gave)? Any pipeline that uses this could use it as a cleanup.

@laurelmay
Copy link
Contributor Author

@wendellpiez If that's something you're able to provide, I'd greatly appreciate it and it'd definitely be a viable workaround for us in the shorter term!

@wendellpiez
Copy link
Contributor

@kylelaker - well in principle such a filter should be simple to make, except there is a detail here that complicates it - in brief, because the bad output has two elements (namely that nm:ERROR, and a catalog), a conformant XML parser will not pass it to an XSLT engine for the fixup I have in mind....

The good news is that the bug is easily fixed, just as you suggest on Line https://github.com/usnistgov/OSCAL/blob/d942435d493578e06507f93cf1b27afd71d2fdf5/xml/convert/oscal_complete_json-to-xml-converter.xsl#L20 if it had

<xsl:if test="matches($file,'\S') and not(unparsed-text-available($file))" expand-text="true">

Or if code intervention is not your thing, we could write a wrapper XSLT that would perform the correction on the result before writing it out (also as a stopgap).

@wendellpiez
Copy link
Contributor

@kylelaker here is a funny idea - what happens if you provide a dummy JSON file reference to the file runtime parameter?

I.e. file=some.json where there is JSON in some.json, but it doesn't matter what.

Meantime this is a straightforward repair and mainly a matter of fitting in.

laurelmay pushed a commit to laurelmay/metaschema that referenced this issue Jul 12, 2023
Previously, if `file` was not specified (and `json` was), an error would
be emitted about the file not being found. This was erroneous as only
the JSON input should have been considered. This applies a small fix to
remediate that, given by Wendell in usnistgov/OSCAL#1849.

I am unsure whether this is the desired long-term fix but I figured
opening a PR to memorialize the patch may be useful.

Co-authored-by: Wendell Piez <[email protected]>
@laurelmay
Copy link
Contributor Author

Okay so I manually modified the "complete" converted to apply the change you provided.

  1. Testing with { "catalog": {} }, I get the expected XML output for the catalog
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json json='{ "catalog": {} }' -o:catalog.xml
    $ cat catalog.xml
    <?xml version="1.0" encoding="UTF-8"?>
    <catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0"/>
    
  2. Testing with a file that doesn't exist, I get an error, though it does have two <nm:ERROR> nodes
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json file=does-not-exist.json -o:catalog.xml
    $ cat catalog.xml
    <?xml version="1.0" encoding="UTF-8"?>
    <nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No file found at does-not-exist.json</nm:ERROR>
    <nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No XPath (XML) JSON found at does-not-exist.json - using syntax of http://www.w3.org/2005/xpath-functions</nm:ERROR>
    
  3. Testing with a file that exists and has non-OSCAL JSON (using https://api.github.com/meta for testing), I get an empty XML document
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json file=gh-meta.json -o:catalog.xml
    $ cat catalog.xml
    <?xml version="1.0" encoding="UTF-8"?>
    
  4. Testing with NIST_SP-800-53_rev5_catalog-min.json using file=
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json file=NIST_SP-800-53_rev5_catalog-min.json -o:catalog.xml
    $ cat catalog.xml | head -n5
    <?xml version="1.0" encoding="UTF-8"?>
    <catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0" uuid="fdac0321-959f-43ec-a91d-322da7d9761c">
       <metadata>
          <title>Electronic Version of NIST SP 800-53 Rev 5 Controls and SP 800-53A Rev 5 Assessment Procedures</title>
          <last-modified>2022-08-23T10:36:49.1330265-04:00</last-modified>
    
  5. Without $json or $file:
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json -o:catalog.xml
    $ cat catalog.xml
    <?xml version="1.0" encoding="UTF-8"?>
    <nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No XPath (XML) JSON found at  - using syntax of http://www.w3.org/2005/xpath-functions</nm:ERROR
    
  6. With empty $json (functionally the same as before)
    $ xslt3 -xsl:oscal_complete_json-to-xml-converter.xsl -t -it:from-json json="" -o:catalog.xml
    $ cat catalog.xml
    <?xml version="1.0" encoding="UTF-8"?>
    <nm:ERROR xmlns:nm="http://csrc.nist.gov/ns/metaschema">No XPath (XML) JSON found at  - using syntax of http://www.w3.org/2005/xpath-functions</nm:ERROR>
    

I've opened usnistgov/metaschema#394 with that patch. I see that there's other work happening to refactor that more generally so I totally understand if it's not merged. But memorializing it makes it a little easier to share exactly how to modify the file for our own needs. Thanks for your help!

@aj-stein-nist
Copy link
Contributor

We will hold on this until after our pending release, review the proposed PR in the updated metaschema repor (metaschema-xslt), and we update submodule in OSCAL repo. We will revisit that potentially in a subsequent sprint depending on when things are ready.

@wendellpiez
Copy link
Contributor

Suggesting this for even greater stress reduction:

In the new package-json-converter.xsl found hereabouts: https://github.com/usnistgov/metaschema-xslt/tree/develop/src/converter-gen line 123

 <XSLT:if test="matches($file, '\S') and not(unparsed-text-available($file))" expand-text="true">
        <XSLT:comment> WARNING: No file found at { $file }</XSLT:comment>
      </XSLT:if>

In addition to not firing wrongly, this makes a warning comment instead of an element at the top level, where it breaks parsers whenever the transformation succeeds in producing results (e.g. via the command line data feed).

@wendellpiez
Copy link
Contributor

The code will shortly be in the new file src/converter-gen/xml-to-json/package-xml-converter.xsl.

Also: consider changing the nm:ERROR elements reported by other top-level issues with inputs into comments, for the same reason?

@wendellpiez
Copy link
Contributor

Actually we didn't do this here - remains as a TODO.

@nikitawootten-nist nikitawootten-nist linked a pull request Jul 25, 2023 that will close this issue
20 tasks
nikitawootten-nist added a commit that referenced this issue Jul 25, 2023
…nverter (#1849)

* Replaced `metaschema` submodule with `metaschema-xslt`
    * The `metaschema-xslt` version also fixes #1849
* Removed all bespoke scripts and replaced them with Makefile targets
* Changed CI infrastructure to use the new Makefile scripts
* Removed Dockerfile and infrastructure as it is no longer needed
* Changed model version to `1.1.0`

Co-authored-by: Wendell Piez <[email protected]>
Co-authored-by: Dmitry Cousin <[email protected]>
Co-authored-by: Chris Compton <[email protected]>
Co-authored-by: A.J. Stein <[email protected]>
nikitawootten-nist added a commit that referenced this issue Jul 25, 2023
…nverter (#1849)

* Replaced `metaschema` submodule with `metaschema-xslt`
    * The `metaschema-xslt` version also fixes #1849
* Removed all bespoke scripts and replaced them with Makefile targets
* Changed CI infrastructure to use the new Makefile scripts
* Removed Dockerfile and infrastructure as it is no longer needed
* Changed model version to `1.1.0`

Co-authored-by: Wendell Piez <[email protected]>
Co-authored-by: Dmitry Cousin <[email protected]>
Co-authored-by: Chris Compton <[email protected]>
Co-authored-by: A.J. Stein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants