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

proposal: encoding/xml: add flag for stricter XML char parsing #69503

Open
maceonthompson opened this issue Sep 17, 2024 · 9 comments
Open

proposal: encoding/xml: add flag for stricter XML char parsing #69503

maceonthompson opened this issue Sep 17, 2024 · 9 comments
Labels
Milestone

Comments

@maceonthompson
Copy link

Proposal Details

Background

As reported by @DemiMarie

The encoding/xml package does not properly validate that the characters within comments, processing instructions, or directives are properly within the CharData range as defined by the XML specification.

Proposal

Add a godebug flag, xmlvalidatechars=1, which enables more strict validation of characters within comments, processing instructions, and directives. It is my understanding that changing XML behavior can sometimes lead to unexpected behavior/breaking changes, but I have tested what would happen if this flag were enabled by default internally and ran into zero issues.

@gopherbot gopherbot added this to the Proposal milestone Sep 17, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@DemiMarie
Copy link
Contributor

I’d prefer if strict validation were on by default, at least eventually if not initially. Would this be possible, or would it break the Go 1 stability guarantee?

@ianlancetaylor
Copy link
Contributor

Making encoding/xml do strict validation by default would be OK with the Go 1 compatibility guarantee. Adding the GODEBUG would give people an easy way to back off.

That said: why would strict validation be a good default? Who would that help in practice?

@ianlancetaylor ianlancetaylor changed the title proposal: encoding/XML: add flag for stricter XML char parsing proposal: encoding/xml: add flag for stricter XML char parsing Sep 18, 2024
@DemiMarie
Copy link
Contributor

Making encoding/xml do strict validation by default would be OK with the Go 1 compatibility guarantee. Adding the GODEBUG would give people an easy way to back off.

That said: why would strict validation be a good default? Who would that help in practice?

XML with malformed characters is ill-formed, so the Principle of Least Astonishment suggests that it should be rejected.

@ianlancetaylor
Copy link
Contributor

XML with malformed characters is ill-formed, so the Principle of Least Astonishment suggests that it should be rejected.

If we were starting from scratch, I would certainly agree. But we aren't. Making this change will most likely break some existing working code, which in an ecosystem that stresses backward compatibility is astonishing in a different way.

@DemiMarie
Copy link
Contributor

XML with malformed characters is ill-formed, so the Principle of Least Astonishment suggests that it should be rejected.

If we were starting from scratch, I would certainly agree. But we aren't. Making this change will most likely break some existing working code, which in an ecosystem that stresses backward compatibility is astonishing in a different way.

Would it be better to create a new XML parsing entry point and mark the old entry points as deprecated? There are a ton of problems in encoding/xml and absolutely strict well-formedness checking might break a lot of code.

@ianlancetaylor
Copy link
Contributor

I think that we should have a plan for encoding/xml/v2. It's clear that there are many problems with encoding/xml. But that is a big undertaking.

In the meantime, we need to decide about this proposal, which is not about v2.

@DemiMarie
Copy link
Contributor

I think that we should have a plan for encoding/xml/v2. It's clear that there are many problems with encoding/xml.

👍

But that is a big undertaking.

💯

In the meantime, we need to decide about this proposal, which is not about v2.

Fair!

You have much more experience than I do when it comes to the Go ecosystem, so I think it would be better for you to make this decision. Other options include adding an extra flag field or setter method.

@aclements
Copy link
Member

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Active
Development

No branches or pull requests

6 participants