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

Fix endless loop issue when Schema tag has no correct xmlns value. #3192

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

WanjohiSammy
Copy link
Member

@WanjohiSammy WanjohiSammy commented Mar 3, 2025

Issues

This pull request fixes #3188.

Description

This PR addresses an issue where an endless loop occurs when the Schema tag does not have xmlns attribute or correct xmlns value. The fix involves adding reader.Skip() before the return statement to ensure the reader skips to the end tag of the current element.

Adding reader.Read() will move the reader to the next element and results to elements with opening and closing tags that do not match.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

Choose a reason for hiding this comment

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

PR Overview

This PR addresses an endless loop issue encountered when the Schema tag has either an invalid or missing xmlns attribute, by adding reader.Skip() calls to properly advance past problematic elements.

  • Fix endless loop by invoking reader.Skip() after reporting unexpected namespaces or elements.
  • Add comprehensive unit tests to verify error reporting when encountering invalid or missing namespace attributes in both Schema and Edmx tags.

Reviewed Changes

File Description
test/UnitTests/Microsoft.OData.Edm.Tests/Csdl/CsdlReaderTests.cs Introduces new test cases to validate error handling for invalid or missing xmlns values.
src/Microsoft.OData.Edm/Csdl/Parsing/Common/XmlDocumentParser.cs Adds reader.Skip() calls following error reporting to prevent endless loops during XML parsing.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Copy link
Contributor

@gathogojr gathogojr left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -148,6 +148,7 @@ internal void ParseDocumentElement()
else
{
this.ReportUnexpectedRootNamespace(this.reader.LocalName, this.DocumentNamespace, expectedNamespaces);
this.reader.Skip();
Copy link
Member

Choose a reason for hiding this comment

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

Can we just tolerate to allow the missing and append it by default by use and only report the wrong namespace used?

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.

Memory Leak in CsdlReader.TryParse Due to missing xmlns=”http://docs.oasis-open.org/odata/ns/edm”
3 participants