Allow StaxEventItemReader to auto-detect the input file encoding

[jdomigon]

Bug description
StaxEventItemReader uses platform default encoding to read XML files.

Environment
Spring-batch version 4.3.0. Related to #807

Steps to reproduce
Just try to parse an XML file with a declared encoding different to the platform encoding.

Expected behavior
Encoding field in StaxEventItemReader should default to null, so that the parser has the opportunity to autodetect the file encoding.

[jdomigon]

Hi @benas. I've checked that just assigning null on the encoding field in StaxEventItemReader allows the eventReader to autodetect the XML file encoding.

[fmbenhassine]

Thank you for checking. However, while reviewing this, I don't see anything wrong with using the platform's default encoding as a default encoding. I believe that is a better default than null.

The title of this issue is implying that the default encoding is used unconditionally, but that is not accurate. It is just a default value, made configurable in #807. If the default encoding is not suitable, it can be changed with the setter added in #807.

Closing this as a non issue.

[jdomigon]

Hi, @fmbenhassine: XML files are a bit different to flat files, as they include a header that declares the encoding of the data that follows.

In my experience it's quite common to pass null as the encoding to the XML parser so that it can decide about it reading the header. Sometimes, it's useful to force the parser to use an encoding when the file to parse doesn't contain the encoding header or when this header is not correct, but this situation is uncommon.

Nevertheless, the configurable encoding introduced in #807, would be enough if the setter would't reject the null value, as it does currently:

	/**
	 * Set encoding to be used for the input file. Defaults to {@link #DEFAULT_ENCODING}.
	 *
	 * @param encoding the encoding to be used
	 */
	public void setEncoding(String encoding) {
		Assert.notNull(encoding, "The encoding must not be null");
		this.encoding = encoding;
	}

[fmbenhassine]

That's a different story. Accepting null and using null as default are different design choices. I am re-opening this issue as we can make the reader accept null, but we need to make sure to address this correctly when configuring the event reader.

In my experience it's quite common to pass null as the encoding to the XML parser so that it can decide about it reading the header

Some StAX implementations might not accept nulls. Spring Batch uses Woodstox, which accepts null encoding, but since the StaxEventItemReader uses the implementation-agnostic XMLInputFactory.createXMLEventReader which does not document how to handle null encodings, I prefer not passing null.

The goal is to cover the option of letting the reader detect the encoding without passing null. I see there are two overloaded XMLInputFactory.createXMLEventReader methods to create the reader:

  /**
   * Create a new XMLEventReader from a java.io.InputStream
   * @param stream the InputStream to read from
   * @return an instance of the {@code XMLEventReader}
   * @throws XMLStreamException if an error occurs
   */
  public abstract XMLEventReader createXMLEventReader(java.io.InputStream stream)
    throws XMLStreamException;

  /**
   * Create a new XMLEventReader from a java.io.InputStream
   * @param stream the InputStream to read from
   * @param encoding the character encoding of the stream
   * @return an instance of the {@code XMLEventReader}
   * @throws XMLStreamException if an error occurs
   */
  public abstract XMLEventReader createXMLEventReader(java.io.InputStream stream, String encoding)
    throws XMLStreamException;

If we make the setter accept null, do you think that xmlInputFactory.createXMLEventReader(inputStream, null) and xmlInputFactory.createXMLEventReader(inputStream) have the same effect WRT detecting the encoding? I will give it a try, but if you can test that and share your feedback, I would be grateful. Using xmlInputFactory.createXMLEventReader(inputStream) is safer and could be added to Spring Batch. The idea is to update the StaxEventItemReader with something like this:

eventReader = this.encoding != null ?  xmlInputFactory.createXMLEventReader(inputStream, this.encoding) :
		xmlInputFactory.createXMLEventReader(inputStream);