snoyberg / xml

Various XML utility packages for Haskell
71 stars 64 forks source link

Add `psIgnoreInternalEntityDeclarations` flag #184

Closed poorlyknitsweater closed 1 year ago

poorlyknitsweater commented 1 year ago

This PR adds psResolveEntities to ParseSettings. It allows users to disable the resolution of (non predefined) entities completely. This is desirable to my use case since the entity resolution has linear complexity even if psEntityExpansionSizeLimit is set to 0.

This is added as a new flag to make it easy to upgrade. If you don't change this flag the behavior stays the same.

k0ral commented 1 year ago

Would it be simpler to add a condition inside parseDoctype to skip entity declarations upfront ? (Cf https://github.com/snoyberg/xml/blob/master/xml-conduit/src/Text/XML/Stream/Parse.hs#L534-L540)

Also, I would suggest renaming psResolveEntities into psIgnoreInternalEntityDeclarations to avoid misleading users into thinking that predefined entities aren't resolved.

poorlyknitsweater commented 1 year ago

Would it be simpler to add a condition inside parseDoctype to skip entity declarations upfront ?

In terms of lines of code, yes. That way we also need only one branch. :+1:

would suggest renaming psResolveEntities into psIgnoreInternalEntityDeclarations

Makes sense to me as well, the original name was chosen as the Python library I'm replacing uses resolve_entities.

poorlyknitsweater commented 1 year ago

Applied your suggestions :+1: