Description
This is a follow-on to work done for #20439 by @hamogu.
In that issue, we discovered that the XML standard excludes reading or writing certain control characters defined here
https://www.w3.org/TR/xml/#NT-Char
Astropy's use of the expat parser (in cextern) excludes those on XML table read (raising an Exception)
|
checkCharRefNumber(int result) { |
|
switch (result >> 8) { |
|
case 0xD8: |
|
case 0xD9: |
|
case 0xDA: |
|
case 0xDB: |
|
case 0xDC: |
|
case 0xDD: |
|
case 0xDE: |
|
case 0xDF: |
|
return -1; |
|
case 0: |
|
if (latin1_encoding.type[result] == BT_NONXML) |
|
return -1; |
|
break; |
|
case 0xFF: |
|
if (result == 0xFFFE || result == 0xFFFF) |
|
return -1; |
|
break; |
|
} |
|
return result; |
|
} |
but our XML writer happily writes those invalid characters out to a file without checking. The result is that astropy will write table/votable XML that cannot be read in by astropy.
Further, the check_token() function modified in #20439 silently strips out non-valid characters, despite the caller that uses it indicating that an Exception is raised. None is. This will silently alter data on the way out. Correction, the return value is never used, so it is a no-op. It runs, but whether it returns True or False is not used for anything. Given how it was wired up, it looks like it was intended to warn the user or raise an error.
|
def check_token(token, attr_name, config=None, pos=None): |
|
""" |
|
Raises a `ValueError` if *token* is not a valid XML token. |
|
|
|
As defined by XML Schema Part 2. |
|
""" |
|
return token is None or xml_check.check_token(token) |
|
def check_token(token): |
|
""" |
|
Returns `True` if *token* is a valid XML token, as defined by XML |
|
Schema Part 2. |
|
""" |
|
return ( |
|
token == "" |
|
or re.match(r"[^\r\n\t ]?([^\r\n\t ]| [^\r\n\t ])*[^\r\n\t ]?$", token) |
|
is not None |
|
) |
Expected behavior
A fix should be something like:
-
Make sure astropy writes valid XML that it can then read in. Data should round-trip or an Exception or Warning should be raised.
-
Don't strip data or fields silently. If for some reason data or a table comes into Astropy Table via some other means (JSON, FITS, ASCII, pandas) and it can't be written out as XML, the user needs to know. It would be very surprising if data was stripped out on roundtrip. Instead, warn or raise an Exception that valid XML cannot be written, and point to exactly what column field or chunk of data is causing the problem.
A precedent for this is lxml, which does exactly this:
ValueError: All strings must be XML compatible: Unicode or ASCII
Astropy pulls in lxml via the [pandas] extra, so unit tests could make sure the behavior is similar.
How to Reproduce
from astropy.io import fits
from astropy.table import Table
# A FITS binary table whose character column holds a form feed (U+000C).
# FITS permits any byte in an 'A' column; XML forbids this one entirely.
fits.BinTableHDU.from_columns(
[fits.Column(name="target", format="10A", array=[b"NGC\x0c1068"])]
).writeto("cat.fits", overwrite=True)
t = Table.read("cat.fits")
print(repr(t["target"][0]))
# 'NGC\x0c1068'
t.write("cat.vot", format="votable", overwrite=True)
# no error, no warning
Table.read("cat.vot", format="votable")
# ValueError: 11:13: not well-formed (invalid token)Versions
import astropy
astropy.system_info()
platform
--------
platform.platform() = 'macOS-26.6.2-arm64-arm-64bit-Mach-O'
platform.version() = 'Darwin Kernel Version 25.6.0: Fri Jul 31 19:16:36 PDT 2026; root:xnu-12377.161.14~5/RELEASE_ARM64_T6030'
platform.python_version() = '3.14.7'
packages
--------
astropy 8.1.0.dev639+ge958a8438
numpy 2.5.2
scipy 1.18.0
matplotlib 3.11.1
pandas 3.0.5
pyerfa 2.0.1.5
Description
This is a follow-on to work done for #20439 by @hamogu.
In that issue, we discovered that the XML standard excludes reading or writing certain control characters defined here
https://www.w3.org/TR/xml/#NT-Char
Astropy's use of the
expatparser (in cextern) excludes those on XML table read (raising an Exception)astropy/cextern/expat/lib/xmltok.c
Lines 1233 to 1254 in e958a84
but our XML writer happily writes those invalid characters out to a file without checking. The result is that astropy will write table/votable XML that cannot be read in by astropy.
Further, theCorrection, the return value is never used, so it is a no-op. It runs, but whether it returns True or False is not used for anything. Given how it was wired up, it looks like it was intended to warn the user or raise an error.check_token()function modified in #20439 silently strips out non-valid characters, despite the caller that uses it indicating that an Exception is raised. None is. This will silently alter data on the way out.astropy/astropy/io/votable/xmlutil.py
Lines 57 to 63 in e958a84
astropy/astropy/utils/xml/check.py
Lines 40 to 49 in e958a84
Expected behavior
A fix should be something like:
Make sure astropy writes valid XML that it can then read in. Data should round-trip or an Exception or Warning should be raised.
Don't strip data or fields silently. If for some reason data or a table comes into Astropy Table via some other means (JSON, FITS, ASCII, pandas) and it can't be written out as XML, the user needs to know. It would be very surprising if data was stripped out on roundtrip. Instead, warn or raise an Exception that valid XML cannot be written, and point to exactly what column field or chunk of data is causing the problem.
A precedent for this is
lxml, which does exactly this:Astropy pulls in
lxmlvia the[pandas]extra, so unit tests could make sure the behavior is similar.How to Reproduce
Versions