Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,25 @@ public interface IXmlStreaming {
/**
* Create a new XML reader with the given input. This reader is generic.
* @param input The text to be parsed
* @param expandEntities If true, entities are directly expanded (throwing errors if not found)
* @param expandEntities Whether entity references should be emitted as distinct events, or
* treated as simple text. In both cases, entities are resolved and their value available via
* [XmlReader.text]. Accessing `text` for an unknown entity will throw an exception. If
* [expandEntities] is false, the name of the entity is available via [XmlReader.localName].
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe mention that "entities in attribute values are always expanded. If the reference in an attribute is unrecognized this will always result in an exception" (new behaviour, old one was an empty string).

For XML wellformedness it is required that entity references reference declared entities. So throwing an exception would be valid (even though the parser is not quite meeting the requirements of even a non-validating parser.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I pushed an update, what do you think?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It looks good. Maybe the test could have a parameter to createReader to determine whether entities are expanded. Alternatively some of the per format subclasses could be duplicated with different factory functions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good - just to clarify, the parameter on createReader should behave the same as the one here right?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Basically yes.

* Note that entities in attributes are always emitted as distinct events, regardless of
* [expandEntities].
* @return A platform independent [XmlReader], generally [nl.adaptivity.xmlutil.core.KtXmlReader]
*/
public fun newGenericReader(input: CharSequence, expandEntities: Boolean = false): XmlReader

/**
* Create a new XML reader with the given input. This reader is generic.
* @param reader The reader/stream to use as input
* @param expandEntities If true, entities are directly expanded (throwing errors if not found)
* @param expandEntities Whether entity references should be emitted as distinct events, or
* treated as simple text. In both cases, entities are resolved and their value available via
* [XmlReader.text]. Accessing `text` for an unknown entity will throw an exception. If
* [expandEntities] is false, the name of the entity is available via [XmlReader.localName].
Comment thread
boswelja marked this conversation as resolved.
* Note that entities in attributes are always emitted as distinct events, regardless of
* [expandEntities].
* @return A platform independent [XmlReader], generally [nl.adaptivity.xmlutil.core.KtXmlReader]
*/
public fun newGenericReader(reader: Reader, expandEntities: Boolean = false): XmlReader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,32 @@ class TestKtXmlReaderExpandEntities : TestCommonReader() {

assertEquals(EventType.END_ELEMENT, r.next())
}

@Test
override fun testInternalEntities() {
val text = """
<!DOCTYPE Test [
<!ENTITY helloWorld "Hello, world!">
<!ENTITY foo "Foo">
<!ENTITY bar "Bar">
<!ENTITY baz "Baz">
]>
<Test>&helloWorld;</Test>
""".trimIndent()
val reader = createReader(text)

var event = reader.next()
if (event == EventType.START_DOCUMENT) event = reader.next()
assertEquals(EventType.DOCDECL, event)
// There should probably be internal entity decl events here, but we only get whitespace...
do {
event = reader.next()
} while (event == EventType.IGNORABLE_WHITESPACE)
// Start element event for <Test>
reader.next()
// Should be the text within the element
event = reader.next()
assertEquals(EventType.TEXT, event)
assertEquals("Hello, world!", reader.text)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,32 @@ class EntityValue311 {
assertContains(e.message ?: "", "unknown")
}

@Test
fun testParseKnownEntityNoResolve() {
val xmlReader = xmlStreaming.newGenericReader(
"""
<!DOCTYPE Test [ <!ENTITY internalEntity "Hello, world!"> ] >
<Test>&internalEntity;</Test>
""".trimIndent(),
false
)
val test = XML.v1.decodeFromReader<ValueTest>(xmlReader)
assertEquals("Hello, world!", test.value)
}

@Test
fun testParseKnownEntityResolve() {
val xmlReader = xmlStreaming.newGenericReader(
"""
<!DOCTYPE Test [ <!ENTITY internalEntity "Hello, world!"> ] >
<Test>&internalEntity;</Test>
""".trimIndent(),
true
)
val test = XML.v1.decodeFromReader<ValueTest>(xmlReader)
assertEquals("Hello, world!", test.value)
}

@Serializable
data class ValueTest(@XmlValue val value: String)
}
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,34 @@ abstract class TestCommonReader {
}
}

protected fun testInternalEntities(createReader: (String) -> XmlReader) {
val text = """
<!DOCTYPE Test [
<!ENTITY helloWorld "Hello, world!">
<!ENTITY foo "Foo">
<!ENTITY bar "Bar">
<!ENTITY baz "Baz">
]>
<Test>&helloWorld;</Test>
""".trimIndent()
val reader = createReader(text)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It may be worthwhile to change this to have a parameter on whether entities are (auto)expanded.


var event = reader.next()
if (event == EventType.START_DOCUMENT) event = reader.next()
assertEquals(EventType.DOCDECL, event)
// There should probably be internal entity decl events here, but we only get whitespace...
do {
event = reader.next()
} while (event == EventType.IGNORABLE_WHITESPACE)
// Start element event for <Test>
reader.next()
// Should be the text within the element
event = reader.next()
assertEquals(EventType.ENTITY_REF, event)
assertEquals("Hello, world!", reader.text)
assertEquals("helloWorld", reader.localName)
}

protected abstract fun createReader(xml: String): XmlReader

@Test
Expand Down Expand Up @@ -339,6 +367,11 @@ abstract class TestCommonReader {
}
}

@Test
open fun testInternalEntities() {
testInternalEntities(::createReader)
}

/**
* Test that triggers invalid handling of `]]>` inside attribute values. #266
*/
Expand Down