1
\$\begingroup\$

I'm writing a script that deals reads UTF-8-encoded XML files and writes parts of those files into a tempfile for further processing.

Sometimes, the input files will have a few malformed characters. If it's just "a few" occurrences, I want those glossed over, but if the file is majorly corrupted, I want to see an error message. I have a solution for this working right now, but I have this nagging feeling that this isn't a clean way to do this.

This creates an example file with malformed data:

f = open("test.xml","w+b")
f.write("\x3c\x61\x3e\x31\x3c\x2f\x61\x3e\x0a\x3c\x62\x3e\x32\xc3\x0a\xbc\x3c\x2f\x62\x3e\x0a")
f.close()

Yes, I know it's probably a stray linebreak that's causing my "malformed utf-8" problem, but let's not make that the issue here.

Now, the following is a simplified version I have in place to deal with malformed utf-8 data up to a certain limit:

import codecs

sourcefile = open("test.xml", "r")

# First line always XML declaration, no special treatment
line = sourcefile.readline()

i = 1
unicodeerror = 0

while line:
    try:
        pos = sourcefile.tell()
        line = sourcefile.readline().decode("utf-8")
    except UnicodeDecodeError as e:
        print "WARNING: Encoding error in line " + str(i+1)
        sourcefile.seek(pos, 0)
        line = sourcefile.readline().decode("utf-8","ignore")
        unicodeerror += 1
    i += 1

if unicodeerror > 5:
    raise StandardError('ERROR: Too many Unicode errors')

As you can see, I read the input file as a byte string, and then read each line individually, doing the UTF-8-Decode within the try-except-clause. That way, I can catch the UnicodeDecodeError and count it. Only after counting it, reread the line, this time ignoring possible mistakes.

However, it "feels wrong" to simply read the file like this instead of doing a codecs.open(FILE, MODE, "utf-8"). With the codecs method, however, I see no way to reread the line with a different error handling directive after I hit the UnicodeDecodeException.

Am I being too cautious? Is this maybe the best / only reliable way to do this? OR is there a much easier solution (which can be achieved within the standard library!)?

\$\endgroup\$
2
\$\begingroup\$

The answer to your question depends on the version of python being used, and exactly what behavior you want. In Python 3.1 and up, the surrogateescape error handler can adeptly handle corrupted files that are otherwise encoded with UTF-8, and even write them back out again. Just use codecs.open(FILE, MODE, "utf-8", "surrogateescape"). If you want to know when invalid bytes were encountered, you can either scan for the relevant surrogate characters (those in the range U+DC80..U+DCFF), or compare to values you get from the replace error handler.

Through Python 2.7, it appears that the surrogateescape error handler is unavailable. You could contrast the results between the ignore and replace error handlers, but there is no convenient way to process the file in Unicode and write it out byte for byte.

In either version, you can theoretically write your own error handler that performs similarly to surrogateescape, ignore or replace, and counts the number of problems it sees. I'm uncertain whether this count can be scoped to a single file, however, as it appears an error handler only receives knowledge of the substring in question when an error is encountered.


On to a code review. Your code feels almost clunky to me. There's a stray import codecs, a lot of repetition (I can't fathom why re-reads the line with an error) and it doesn't try to leverage iterators. Perhaps you're on a particularly old version of Python (no, you use as in the except line), or perhaps you just haven't worked with iterators much. Probably most of this was from trying to extract the core issue, but I can only review it as is. I apologize for any errors in my code that follows; I haven't tested it, but I am willing to update it.

I would be strongly tempted to replace most of your code with something like this, and that's before trying to leverage anything I talked about above:

unicodeerror = 0
for i, line in enumerate(open("test.xml", "r")):
    try:
        line.decode("utf-8")
    except UnicodeDecodeError:
        print "WARNING: Encoding error in line", i+1
        unicodeerror += 1

if unicodeerror > 5:
    raise StandardError("ERROR: Too many Unicode errors")

If you need to do more, such as actually process the line, then it's reasonable to save the results of the decode, and to try again with an error handler in the except clause:

    try:
        text = line.decode("utf-8")
    except UnicodeDecodeError:
        # error tracking omitted
        text = line.decode("utf-8", "ignore")
    process(text)

If you wanted to integrate a comparison between two error handlers, it would replace the exception, so you could do something like this:

import codecs
unicodeerror = 0
for i, (ignored, replaced) in enumerate(zip(
    codecs.open("test.xml", "r", "utf-8", "ignore"),
    codecs.open("test.xml", "r", "utf-8", "replace"))):
    if ignored != replaced:
        print "WARNING: Encoding error in line", i+1
        unicodeerror += 1
if unicodeeror > 5:
    raise StandardError("ERROR: Too many Unicode errors")

And I'll leave a custom error handler totally up to you. If it works, it would probably be a lot more efficient than this approach, but there's something to be said for the simplicity of the visible code.


As a side comment, while I found this question interesting enough to delve into, the more I did so, the more it felt like a question about the right way to do this than a request to review your code. Keep in mind that it's much more on topic here to review code and make suggestions on how to improve it. So this particular approach isn't necessarily a great fit for codereview.SE.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the competent and patient reply! To address your last issue first: I was unsure whether that went here or ... where else? SE central? I don't want to clutter the Code Review space with problem that don't belong here, so sorry for that. \$\endgroup\$ – Thor Sep 10 '14 at 16:32
  • \$\begingroup\$ I went the readline-way only because when I first wrote this, I still used the strict error handler of the codecs.open method, and I didn't see a way of rereading the line with another error handler after catching the Unicode exception (yes, I do need the data in that line). I do like your last suggestion, using two file handles with contrasting error handlers. This has indeed improved the readability of my code loads, so thanks for that! \$\endgroup\$ – Thor Sep 10 '14 at 16:50
  • \$\begingroup\$ Thanks for the interesting scenario! Thinking later I realized that with large files it may be important to change zip to itertools.izip, so keep that in mind too. \$\endgroup\$ – Michael Urman Sep 10 '14 at 23:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.