Uploaded image for project: 'logback'
  1. logback
  2. LOGBACK-1257

Invalid files in case of append=true and Encoder with non-null headerBytes() / footerBytes()

    XMLWordPrintable

Details

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • 1.5.7
    • 1.2.0
    • logback-core
    • None
    • any

    Description

      I just upgraded Lilith to Logback 1.2.0.

      I previously had the following code in my Encoder implementations:

      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();
      }
      

      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 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.

      Attachments

        Activity

          People

            ceki Ceki Gülcü
            jhuxhorn Joern Huxhorn
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated: