pugixml icon indicating copy to clipboard operation
pugixml copied to clipboard

xml_document::save_file may silently fail and corrupt data

Open bgs99 opened this issue 3 years ago • 1 comments

xml_document::save_file calls impl::save_file_impl which attempts to catch write errors by calling ferror: https://github.com/zeux/pugixml/blob/521b2cd854f8d65f173107d056d2b9c6d49b6563/src/pugixml.cpp#L5066-L5074

However, because file writes via fwrite are buffered in most implementations, the final write on disk may happen when the file is flushed on close. Any error possibly happening there is silently ignored.

The following example code will create an empty file (or truncate an existing one) if there's no disk space on the partition, while printing "success":

#include "pugixml.hpp"

#include <iostream>

int main()
{
    pugi::xml_document document;
    pugi::xml_node decl = document.prepend_child(pugi::node_declaration);
    decl.append_attribute("version").set_value("1.0");
    decl.append_attribute("encoding").set_value("utf-8");

    const bool success = document.save_file("file.xml");

    if (success)
    {
        std::cout << "success" << std::endl;
    }
    else
    {
        std::cout << "failure" << std::endl;
    }

    return 0;
}

The expected behavior for this code in that setup is to return false from xml_document::save_file and print "failure".

Manually calling fflush in impl::save_file_impl before calling ferror should fix this problem.

bgs99 avatar Sep 15 '22 15:09 bgs99

Yeah that makes sense. I believe it's also the case that writing metadata might fail during fclose, so ideally we should check the result of fclose instead.

zeux avatar Sep 15 '22 18:09 zeux