Skip to content

Refactor NLAP XML parsing to a new Xerces-based tree parser with static DTD validation - #202

Draft
clauspruefer with Copilot wants to merge 5 commits into
mainfrom
copilot/refactor-xml-parser-new
Draft

Refactor NLAP XML parsing to a new Xerces-based tree parser with static DTD validation#202
clauspruefer with Copilot wants to merge 5 commits into
mainfrom
copilot/refactor-xml-parser-new

Conversation

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

This PR replaces the XML parser implementation in lib/xml with a clean-slate design aligned to the NLAP parser contract: Xerces-backed parsing, DTD-constrained validation, strict framing/syntax/content errors, and zero-copy leaf addressing from the caller buffer.
The parser now returns per-message nested result trees (vector<unordered_map<...>>) with terminal Address/Length spans suitable for direct field extraction.

  • Parser API + data model

    • Replaced legacy request-property model with a nested tree model:
      • ParseResult_t { vector<ResultTree_t> Results, uint16_t ErrorCode }
      • leaf nodes carry const char* Address + size_t Length
    • Input contract is now parse(char* InputBuffer) with internal std::string_view handling.
    • Added runtime parse buffer control (setParseBufferSize, getParseBufferSize).
  • Fresh Xerces parsing pipeline

    • Reimplemented message parsing around Xerces DOM from scratch.
    • Separated framing/splitting from XML parsing logic.
    • Added coroutine/yield-based sub-routines (generator-style) for message slicing and DOM child iteration.
  • Validation + error semantics

    • Enforced new constants in xmlconstants.hpp:
      • XML_ERROR_INVALID_CONTENT_DTD
      • XML_ERROR_INVALID_SYNTAX
      • XML_ERROR_INVALID_FRAMING
    • Implemented strict multi-message framing checks (reject non-XML data between messages).
  • Static DTD embedding

    • Added lib/xml/static-dtd.hpp with constexpr hex-encoded DTD bytes.
    • Grammar is loaded from memory (MemBufInputSource) and used as cached DTD for validation.
  • Security posture

    • Added explicit external-entity blocking resolver during parse passes.
    • Allows only NLAP DTD resolution path while blocking other external entities.
  • XML module reset + test surface update

    • Replaced lib/xml/xmlparser.hpp/.cpp implementation and updated XML module behavior accordingly.
    • Reworked XML-focused test coverage to match new output/error contracts (unit, end-to-end, fuzz-style, minor perf scopes).
char InputBuffer[4096] = "<NLAP><Request>...</Request></NLAP>";
XMLParser Parser(4096);

const ParseResult_t Req = Parser.parse(InputBuffer);
const XMLNode& URLNode = Req.Results.at(0).at("NLAP").at("Request").at("Header").at("URL");

const char* URLAddress = URLNode.Address;
size_t URLLength = URLNode.Length;

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@clauspruefer clauspruefer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot who the fuck did tell you to use coroutines? I will not pay this shit! Please start over with the exact specifications i gave you!

Copilot AI review requested due to automatic review settings July 25, 2026 11:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot who the fuck did tell you to use coroutines? I will not pay this shit! Please start over with the exact specifications i gave you!

Done. I removed the coroutine-based implementation and replaced it with straightforward loop/vector logic in commit 2c854ef, keeping the parser behavior and tests intact.

@clauspruefer clauspruefer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot From the original specifications 5. Use std::generatorand co_yieldin sub-parser routines 6. Usestd::string_viewto reduce copy operations 7. Use smart pointersunique_ptr<>andshared_ptr<> where possible In your last commit you used C++20 Coroutines, it was my fault it must be C++23 std::generator and only co_yield, please re-aplly these specs, use std::generator and co_yield, recheck using std::string_view where possible and use smart pointers! And always produce clean OOP code!

Copilot AI review requested due to automatic review settings July 25, 2026 11:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot From the original specifications 5. Use std::generatorand co_yieldin sub-parser routines 6. Usestd::string_viewto reduce copy operations 7. Use smart pointersunique_ptr<>andsh...

Done — reapplied the parser specs in commit d07c727: C++23 std::generator + co_yield in sub-parser routines, retained std::string_view slicing, and tightened smart-pointer ownership usage in the XML parser.

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.

3 participants