[logback-dev] [JIRA] (LOGBACK-1257) Invalid files in case of append=true and Encoder with non-null headerBytes() / footerBytes()

[QOS.CH (JIRA)]

Joern Huxhorn created LOGBACK-1257:
--------------------------------------

             Summary: Invalid files in case of append=true and Encoder with non-null headerBytes() / footerBytes()
                 Key: LOGBACK-1257
                 URL: https://jira.qos.ch/browse/LOGBACK-1257
             Project: logback
          Issue Type: Bug
          Components: logback-core
    Affects Versions: 1.2.0
         Environment: any
            Reporter: Joern Huxhorn
            Assignee: Logback dev list


I just [upgraded Lilith to Logback 1.2.0|https://github.com/huxi/lilith/commit/52ebceb1ea748760e5e2e665a6986536f0422a0c].

I previously had the following code in my {{Encoder}} implementations:
{code:java}
public void init(OutputStream os) throws IOException
{
    super.init(os);
    if(os instanceof ResilientFileOutputStream)
    {
        ResilientFileOutputStream rfos = (ResilientFileOutputStream) os;
        File file = rfos.getFile();
        if(file.length() == 0)
        {
            // write header
            Map<String, String> metaDataMap = new HashMap<>();
            metaDataMap.put(FileConstants.CONTENT_TYPE_KEY, FileConstants.CONTENT_TYPE_VALUE_ACCESS);
            metaDataMap.put(FileConstants.CONTENT_FORMAT_KEY, FileConstants.CONTENT_FORMAT_VALUE_PROTOBUF);
            metaDataMap.put(FileConstants.COMPRESSION_KEY, FileConstants.COMPRESSION_VALUE_GZIP);
            writeHeader(metaDataMap);
        }
    }
    else
    {
        throw new IOException("OutputStream wasn't instanceof ResilientFileOutputStream! "+os);
    }
    wrappingEncoder.reset();
}
{code}

This ensured that the header bytes would only be written once in case of an empty file and not again in the case where events would be appended to an already existing file. We [apparently discussed about this|https://github.com/huxi/lilith/commit/4e0d5d7ecf7b1e102f6ac276bda1247f61fb9bc1] nearly seven years ago.

Given that I can't do anything like this on my side anymore, I'd argue that logic like this would now have to be implemented in {{OutputStreamAppender}}, {{RollingFileAppender}} or {{FileAppender}}. I suspect {{FileAppender}} would be the correct place but {{encoder.headerBytes()}} is currently called exclusively in {{OutputStreamAppender}}.

Nothing special would need to be done in case of {{append=false}}:
* write header (if available) when the appender is started
* write events
* write footer (if available) when the appender is stopped.

In case of {{append=true}}, behavior would have to be different, though:
* write header when the appender is started, but only if the file was still empty, otherwise do nothing
* write events
* do nothing when the appender is stopped, i.e. *don't* write the footer!

This would be the correct behavior in case of binary files (like Lilith log files), HTML (a browser could live with malformed content but resulting output could look strange/nested) and XML (an XML parser could cope with a missing closing tag at the end of the file by simply stopping but couldn't cope with a second or third XML root element).

In the absence of a real-life counter example I'd argue that the above logic would be universally correct. Otherwise, behavior would need to be configurable in some way, with reasonable defaults in place.



--
This message was sent by Atlassian JIRA
(v7.3.1#73012)