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

encoding/xml: empty namespace prefix definitions should be illegal #8068

Open
ghost opened this issue May 21, 2014 · 12 comments
Open

encoding/xml: empty namespace prefix definitions should be illegal #8068

ghost opened this issue May 21, 2014 · 12 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ghost
Copy link

ghost commented May 21, 2014

What does 'go version' print?
go version go1.2 linux/amd64


What steps reproduce the problem?
http://play.golang.org/p/f8UWfdF7Hb

What happened?
No-error.

What should have happened instead?
Should generate an error.

Per http://www.w3.org/TR/REC-xml-names/#dt-prefix a prefix must refer to a namespace
name, not the empty string. 

Please provide any additional information below.

Ran into this in implementing webdav, the litmus tests test this error condition [and
due to xml.Token hiding prefixing it is impossible to detect otherwise]
@griesemer
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-none.

@rogpeppe
Copy link
Contributor

Comment 2:

> Per http://www.w3.org/TR/REC-xml-names/#dt-prefix a prefix must
> refer to a namespace name, not the empty string. 
I couldn't find this stated clearly in the spec. It says "The attribute's normalized
value MUST be either a URI reference — the namespace name identifying the namespace
— or an empty string.". It also explicitly allows the empty string for the xmlns
attribute itself (from section 6.2: "The attribute value in a default namespace
declaration MAY be empty. This has the same effect, within the scope of the declaration,
of there being no default namespace.")
That said, it's not clear what the semantics should be for an empty value.

@ghost
Copy link
Author

ghost commented Nov 27, 2014

Comment 3:

I must admit I was echo'ing the complaint from the WebDAV litmus test suite; which
claimed an empty prefix specification was invalid. 
I agree with your reading that it doesn't appear clear. 6.3. doesn't apply here, i'm
concerned specifically with namespace prefixes [the test in WebDAV is with xmlns:dav=""]

@ghost ghost added new labels Nov 27, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@nigeltao
Copy link
Contributor

It could be that, at the time of writing of the WebDAV litmus test suite, an empty namespace prefix was illegal, but that has since become legal. The grammar differs between
http://www.w3.org/TR/2006/REC-xml-names-20060816/#ns-decl and
http://www.w3.org/TR/REC-xml-names/#dt-prefix

@gopherbot
Copy link

Change https://golang.org/cl/105636 mentions this issue: encoding/xml: fix invalid empty namespace without prefix

@iwdgo
Copy link
Contributor

iwdgo commented Apr 8, 2018

Namespace handling is not checking this case before adding the namespace to the namespace array of the decoder. A related fix is submitted which also rejects syntax xmlns:="..." which is invalid.

@gopherbot
Copy link

Change https://golang.org/cl/109855 mentions this issue: encoding/xml : Fixes to enforce XML namespace standard

ydnar pushed a commit to ydnar/go that referenced this issue Sep 27, 2021
All issues of 13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml : fix duplication of namespace tags by encoder (7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml : fix unexpected behavior of encoder.Indent("", "") (13185, 11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml : fix overriding by empty namespace (7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml : fix panic on embedded unexported XMLName (10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml : fix panic of unmarshaling of anonymous structs (16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml : fix closing tag failure (20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml : add check of namespaces to detect field names conflicts (8535, 11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml : fix normalization of attributes values (20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (20396)

The occurrence of second : in space and tag name is rejected.

Fixes : golang#7113, golang#7535, golang#8068,  golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165
ydnar pushed a commit to ydnar/go that referenced this issue Sep 27, 2021
All issues of 13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml : fix duplication of namespace tags by encoder (7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml : fix unexpected behavior of encoder.Indent("", "") (13185, 11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml : fix overriding by empty namespace (7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml : fix panic on embedded unexported XMLName (10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml : fix panic of unmarshaling of anonymous structs (16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml : fix closing tag failure (20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml : add check of namespaces to detect field names conflicts (8535, 11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml : fix normalization of attributes values (20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (20396)

The occurrence of second : in space and tag name is rejected.

Fixes : golang#7113, golang#7535, golang#8068,  golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165
ydnar added a commit to ydnar/go that referenced this issue Oct 12, 2021
All issues of golang#13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml: fix duplication of namespace tags by encoder (golang#7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml: fix unexpected behavior of encoder.Indent("", "") (golang#13185, golang#11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml: fix overriding by empty namespace (golang#7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml: fix panic on embedded unexported XMLName (golang#10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml: fix panic of unmarshaling of anonymous structs (golang#16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml: fix closing tag failure (golang#20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml: add check of namespaces to detect field names conflicts (golang#8535, golang#11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (golang#8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml: fix normalization of attributes values (golang#20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (golang#20396)

The occurrence of second : in space and tag name is rejected.

Fixes: golang#7113, golang#7535, golang#8068, golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165

Commit originally authored by: Constantin Konstantinidis <constantinkonstantinidis@gmail.com>

encoding/xml: fix printing of namespace prefix in tag names

Prefix displays in XML when defined in tag names.
The URL of the name space is also returned for an End Token as
documentation requires to improve idempotency of Marshal/Unmarshal.

Translating the prefix is popping the NS which was unavailable
for translation. Translate for an End Token has been moved inside
the pop element part.

Fixes golang#9519

Change-Id: Id7a14d07c106a76a487b5c4e28d9d563fe061c60

encoding/xml: restore changes lost in merge

See golang#43168.

encoding/xml: gofmt

encoding/xml: minimalIndent -> minIndent to shrink diff

encoding/xml: gofmt

encoding/xml: revert changes to (*Decoder).attrval from master

encoding/xml: restore (*printer).writeIndent to master

encoding/xml: remove comments that don’t change functionality of tests
@gopherbot
Copy link

Change https://golang.org/cl/355353 mentions this issue: encoding/xml: support xmlns prefixes

ydnar added a commit to ydnar/go that referenced this issue Oct 12, 2021
All issues of golang#13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml: fix duplication of namespace tags by encoder (golang#7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml: fix unexpected behavior of encoder.Indent("", "") (golang#13185, golang#11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml: fix overriding by empty namespace (golang#7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml: fix panic on embedded unexported XMLName (golang#10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml: fix panic of unmarshaling of anonymous structs (golang#16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml: fix closing tag failure (golang#20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml: add check of namespaces to detect field names conflicts (golang#8535, golang#11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (golang#8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml: fix normalization of attributes values (golang#20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (golang#20396)

The occurrence of second : in space and tag name is rejected.

Fixes: golang#7113, golang#7535, golang#8068, golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165

Commit originally authored by: Constantin Konstantinidis <constantinkonstantinidis@gmail.com>

encoding/xml: fix printing of namespace prefix in tag names

Prefix displays in XML when defined in tag names.
The URL of the name space is also returned for an End Token as
documentation requires to improve idempotency of Marshal/Unmarshal.

Translating the prefix is popping the NS which was unavailable
for translation. Translate for an End Token has been moved inside
the pop element part.

Fixes golang#9519

Change-Id: Id7a14d07c106a76a487b5c4e28d9d563fe061c60

encoding/xml: restore changes lost in merge

See golang#43168.

encoding/xml: gofmt

encoding/xml: minimalIndent -> minIndent to shrink diff

encoding/xml: gofmt

encoding/xml: revert changes to (*Decoder).attrval from master

encoding/xml: restore (*printer).writeIndent to master

encoding/xml: remove comments that don’t change functionality of tests

encoding/xml: use t.Errorf instead of fmt.Errorf

encoding/xml: fix test case for golang#11496
ydnar added a commit to ydnar/go that referenced this issue Oct 14, 2021
All issues of golang#13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml: fix duplication of namespace tags by encoder (golang#7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml: fix unexpected behavior of encoder.Indent("", "") (golang#13185, golang#11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml: fix overriding by empty namespace (golang#7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml: fix panic on embedded unexported XMLName (golang#10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml: fix panic of unmarshaling of anonymous structs (golang#16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml: fix closing tag failure (golang#20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml: add check of namespaces to detect field names conflicts (golang#8535, golang#11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (golang#8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml: fix normalization of attributes values (golang#20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (golang#20396)

The occurrence of second : in space and tag name is rejected.

Fixes: golang#7113, golang#7535, golang#8068, golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165

Commit originally authored by: Constantin Konstantinidis <constantinkonstantinidis@gmail.com>

encoding/xml: fix printing of namespace prefix in tag names

Prefix displays in XML when defined in tag names.
The URL of the name space is also returned for an End Token as
documentation requires to improve idempotency of Marshal/Unmarshal.

Translating the prefix is popping the NS which was unavailable
for translation. Translate for an End Token has been moved inside
the pop element part.

Fixes golang#9519

Change-Id: Id7a14d07c106a76a487b5c4e28d9d563fe061c60

encoding/xml: restore changes lost in merge

See golang#43168.

encoding/xml: gofmt

encoding/xml: minimalIndent -> minIndent to shrink diff

encoding/xml: gofmt

encoding/xml: revert changes to (*Decoder).attrval from master

encoding/xml: restore (*printer).writeIndent to master

encoding/xml: remove comments that don’t change functionality of tests

encoding/xml: use t.Errorf instead of fmt.Errorf

encoding/xml: fix test case for golang#11496

encoding/xml: revert unrelated change in (*Decoder).readName

encoding/xml: remove comments
ydnar added a commit to ydnar/go that referenced this issue Oct 14, 2021
All issues of golang#13400 which are not new functionalities have fixes.
There are minor incompatibilities between them due to the handling of
prefixes. Duplicating a prefix or an namespace is invalid XML.
This is now avoided. XML produced is always valid.

Tests have been added for each fix and example and previous tests
fixed as output is already more compact is some cases.

encoding/xml: fix duplication of namespace tags by encoder (golang#7535)

A tag prefix identifies the name space of the tag and not the default name
space like xmlns="...". Writing the prefix is incorrect when it is bound
to a name space using the standard xmlns:prefix="..." attribute.
This fix skips this print.

Consequences are
 - duplication is avoided in line with name space standard in reference.
 - the _xmlns declaration does not appear anymore. To keep the previous
behaviour, the prefix is printed in all other cases. Token now always
produces well-formed XML except when the explicit name space collides
with the prefix.

Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has
be removed in all wants of tests. In some cases, useless declarations like
xmlns:x="x" are still added in line with previous behavior.

encoding/xml: fix unexpected behavior of encoder.Indent("", "") (golang#13185, golang#11431)

MarshalIndent and Marshal share code. When prefix and indent are
empty, the behavior is like Marshal when it should have a minimal
indent, i.e. new line as in documentation.

A boolean is added to the local printer struct which defaults to false.
It is set to true only when MarshalIndent is used and prefix and indent
are empty.

encoding/xml: fix overriding by empty namespace (golang#7113)

The namespace defined by xmlns="value" can be overridden in
every included tag including by the empty namespace xmlns="".
Empty namespace is not authorized with a prefix (xmlns:ns="").

Method to calculate indent of XML handles depth of tag and
its associated namespace. The fix leaves the method active even
when no indent is required.

An XMLName field in a struct means that namespace must be
enforced even if empty. This occurs only on an inner tag as an
overwrite of any non-empty namespace of its outer tag.

To obtain the xmlns="" required, an attribute is added.

encoding/xml: fix panic on embedded unexported XMLName (golang#10538)

By laws of reflection, unexported fields are unreachable by .Value.
XMLName are allowed at any level of an inner struct but the struct
may not be reachable. If XMLName field is found without name,
it cannot be exported and the value is discarded like other fields.
Some comments have been to underline where the issue arises.
Another XMLName test was incorrectly set up and is fixed.
Various cases added in a specific test.

encoding/xml: fix panic of unmarshaling of anonymous structs (golang#16497)

Encoding/xml is using type "typinfo" which provides its own "value" method
using the reflection package. It fails to check for the documented possible
panics of the reflection methods. It is impossible to fix the method as the
parameter is also using reflection.

The fix is to discard anonymous structs which have no value anyway.
Encoder/xml documentation already mentions that fields are accessed using
the reflection package. A relevant test is added.

encoding/xml: fix closing tag failure (golang#20685)

Push/pop of elements name must be done using the eventual prefix together
with the tag name. The operation is moved before the translation of prefix
into its URI. One end element of a test is fixed as expecting the last used
namespace is incorrect.
After closing a tag using a namespace, the valid namespace must be taken
from the opening tag.

encoding/xml: add check of namespaces to detect field names conflicts (golang#8535, golang#11724)

Comparing namespaces of fields was missing and false conflicts were detected.

encoding/xml: fix invalid empty namespace without prefix (golang#8068)

Empty namespace is allowed only if it has no prefix. An error message is now
returned if a prefix exists. A similar case when no prefix is provided,
thus with syntax xmlns:="..." is also rejected.

encoding/xml: fix normalization of attributes values (golang#20614)

The attribute value was read as text. The existing attribute reader logic is
fixed as an attribute may have a namespace or only a prefix. Other
possibilities have been removed.

To keep the behavior of raw token which allows many faults in attributes list,
error handling is heavily using the Strict parameter of the decoder.
Specific tests have been added including list of attributes.

To keep the behavior or unmarshal, escaped characters handling has been
added but it is not symmetrical to Marshal for quotes but follows XML
specification.

encoding/xml: fix absence of detection of another : in qualified names (golang#20396)

The occurrence of second : in space and tag name is rejected.

Fixes: golang#7113, golang#7535, golang#8068, golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165

Commit originally authored by: Constantin Konstantinidis <constantinkonstantinidis@gmail.com>

encoding/xml: fix printing of namespace prefix in tag names

Prefix displays in XML when defined in tag names.
The URL of the name space is also returned for an End Token as
documentation requires to improve idempotency of Marshal/Unmarshal.

Translating the prefix is popping the NS which was unavailable
for translation. Translate for an End Token has been moved inside
the pop element part.

Fixes golang#9519

Change-Id: Id7a14d07c106a76a487b5c4e28d9d563fe061c60

encoding/xml: restore changes lost in merge

See golang#43168.

encoding/xml: gofmt

encoding/xml: minimalIndent -> minIndent to shrink diff

encoding/xml: gofmt

encoding/xml: revert changes to (*Decoder).attrval from master

encoding/xml: restore (*printer).writeIndent to master

encoding/xml: remove comments that don’t change functionality of tests

encoding/xml: use t.Errorf instead of fmt.Errorf

encoding/xml: fix test case for golang#11496

encoding/xml: revert unrelated change in (*Decoder).readName

encoding/xml: remove comments

encoding/xml: remove comments

encoding/xml: edit comments
@heschi
Copy link
Contributor

heschi commented Nov 29, 2022

As a heads up, this fix broke a couple of Google-internal tests that process XML from various sources. I believe that the added check is probably correct, but it seems very possible that this will cause some trouble. Should there be a way to opt out? Given that the fix waited 8 years, perhaps rolling back and taking some time to think about it would be reasonable?

@dmitshur dmitshur modified the milestones: Unplanned, Go1.20 Nov 29, 2022
@ianlancetaylor
Copy link
Contributor

At this stage in the release cycle we should roll back and try again next cycle. I will send a revert CL.

@ianlancetaylor
Copy link
Contributor

Revert CL is 453996, reopening this issue.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.20, Go1.21 Nov 29, 2022
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 29, 2022
@gopherbot
Copy link

Change https://go.dev/cl/453996 mentions this issue: Revert "encoding/xml: disallow empty namespace when prefix is set"

gopherbot pushed a commit that referenced this issue Nov 29, 2022
This reverts CL 105636.

Reason for revert: Fails with existing XML data.  At this stage in the release cycle we should revert, and try again next time with some way to support existing XML.

For #8068

Change-Id: Ia84cbf3a84878ac7190f72998545dee22c36c45e
Reviewed-on: https://go-review.googlesource.com/c/go/+/453996
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

10 participants