Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
*/
package org.apache.avro.file;

import java.io.IOException;
import java.io.Closeable;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
Expand All @@ -27,12 +27,11 @@
import java.util.Map;

import org.apache.avro.InvalidAvroMagicException;
import org.apache.avro.JsonSchemaParser;
import org.apache.avro.Schema;
import org.apache.avro.UnknownAvroCodecException;
import org.apache.avro.io.BinaryDecoder;
import org.apache.avro.io.DatumReader;
import org.apache.avro.io.DecoderFactory;
import org.apache.avro.io.BinaryDecoder;

/** Read files written by Avro version 1.2. */
public class DataFileReader12<D> implements FileReader<D>, Closeable {
Expand Down Expand Up @@ -89,7 +88,7 @@ public DataFileReader12(SeekableInput sin, DatumReader<D> reader) throws IOExcep
if (codec != null && !codec.equals(NULL_CODEC)) {
throw new UnknownAvroCodecException("Unknown codec: " + codec);
}
this.schema = JsonSchemaParser.parseInternal(getMetaString(SCHEMA));
this.schema = parseSchema();
this.reader = reader;

reader.setSchema(schema);
Expand All @@ -116,6 +115,10 @@ public synchronized long getMetaLong(String key) {
return Long.parseLong(getMetaString(key));
}

private Schema parseSchema() throws IOException {
return DataFileStream.parseSchemaFromMetadata(getMetaString(SCHEMA), SCHEMA);
}

/** Return the schema used in this file. */
@Override
public Schema getSchema() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void initialize(InputStream in, byte[] magic) throws IOException {

// finalize the header
header.metaKeyList = Collections.unmodifiableList(header.metaKeyList);
header.schema = JsonSchemaParser.parseInternal(getMetaString(DataFileConstants.SCHEMA));
header.schema = parseHeaderSchema();
this.codec = resolveCodec();
reader.setSchema(header.schema);
}
Expand Down Expand Up @@ -198,6 +198,21 @@ public long getMetaLong(String key) {
return Long.parseLong(getMetaString(key));
}

static Schema parseSchemaFromMetadata(String schemaJson, String schemaMetadataKey) throws IOException {
if (schemaJson == null) {
throw new IOException("Missing required metadata: " + schemaMetadataKey);
}
try {
return JsonSchemaParser.parseInternal(schemaJson);
} catch (AvroRuntimeException e) {
throw new IOException("Invalid schema in metadata: " + schemaMetadataKey, e);
}
}

private Schema parseHeaderSchema() throws IOException {
return parseSchemaFromMetadata(getMetaString(DataFileConstants.SCHEMA), DataFileConstants.SCHEMA);
}

/**
* Returns an iterator over entries in this file. Note that this iterator is
* shared with other users of the file: it does not contain a separate pointer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,27 @@
import java.io.FileInputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.lang.management.ManagementFactory;
import java.lang.management.OperatingSystemMXBean;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import com.sun.management.UnixOperatingSystemMXBean;
import org.apache.avro.file.DataFileReader;
import org.apache.avro.file.DataFileConstants;
import org.apache.avro.file.DataFileReader12;
import org.apache.avro.file.DataFileStream;
import org.apache.avro.file.DataFileWriter;
import org.apache.avro.file.FileReader;
import org.apache.avro.file.SeekableByteArrayInput;
import org.apache.avro.file.SeekableFileInput;
import org.apache.avro.file.SeekableInput;
import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.generic.GenericDatumWriter;
import org.apache.avro.io.BinaryEncoder;
import org.apache.avro.io.EncoderFactory;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

Expand Down Expand Up @@ -236,4 +244,79 @@ void invalidMagicBytes() throws IOException {
() -> DataFileReader.openReader(fileInput, new GenericDatumReader<>()));
}
}

@Test
void missingSchemaMetadataDoesNotThrowNullPointerException() throws IOException {
byte[] malformedFile = buildContainerHeaderWithoutSchema();

IOException streamException = assertThrows(IOException.class,
() -> new DataFileStream<>(new ByteArrayInputStream(malformedFile), new GenericDatumReader<>()));
assertNotNull(streamException.getMessage());
assertTrue(streamException.getMessage().contains(DataFileConstants.SCHEMA));

IOException readerException = assertThrows(IOException.class,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned DataFileReader12 in the PR (and the problem was indeed fixed there too), does this test case give sufficient coverage to verify those changes too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - the existing test only covers DataFileStream and DataFileReader (current format). It does not exercise DataFileReader12 since that class uses a different binary layout (footer-based metadata).

I've now added a dedicated test (missingSchemaMetadataInVersion12DoesNotThrowNullPointerException) that constructs a minimal Avro 1.2 format container with the sync marker but no schema entry, and asserts that DataFileReader12 throws a descriptive IOException rather than an NPE.

() -> new DataFileReader<>(new SeekableByteArrayInput(malformedFile), new GenericDatumReader<>()));
assertNotNull(readerException.getMessage());
assertTrue(readerException.getMessage().contains(DataFileConstants.SCHEMA));
}

private static byte[] buildContainerHeaderWithoutSchema() throws IOException {
ByteArrayOutputStream output = new ByteArrayOutputStream();
output.write(DataFileConstants.MAGIC);

BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(output, null);
encoder.writeMapStart();
encoder.setItemCount(1);
encoder.startItem();
encoder.writeString(DataFileConstants.CODEC);
encoder.writeBytes("null".getBytes(StandardCharsets.UTF_8));
encoder.writeMapEnd();
Comment thread
iemejia marked this conversation as resolved.
encoder.writeFixed(new byte[DataFileConstants.SYNC_SIZE]);
encoder.flush();

return output.toByteArray();
}

@Test
void missingSchemaMetadataInVersion12DoesNotThrowNullPointerException() throws IOException {
byte[] malformedFile = buildVersion12ContainerWithoutSchema();

IOException exception = assertThrows(IOException.class,
() -> new DataFileReader12<>(new SeekableByteArrayInput(malformedFile), new GenericDatumReader<>()));
assertNotNull(exception.getMessage());
assertTrue(exception.getMessage().contains("schema"));
}

/**
* Builds a minimal Avro 1.2 format container with the footer metadata map
* containing only a sync marker but no schema entry.
*/
private static byte[] buildVersion12ContainerWithoutSchema() throws IOException {
ByteArrayOutputStream output = new ByteArrayOutputStream();
// Avro 1.2 magic: 'O' 'b' 'j' 0x00
output.write(new byte[] { (byte) 'O', (byte) 'b', (byte) 'j', 0 });

// Write the footer (metadata map with sync but no schema)
ByteArrayOutputStream footer = new ByteArrayOutputStream();
BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(footer, null);
encoder.writeMapStart();
encoder.setItemCount(1);
encoder.startItem();
encoder.writeString("sync");
encoder.writeBytes(new byte[16]); // 16-byte sync marker
encoder.writeMapEnd();
encoder.flush();

byte[] footerBytes = footer.toByteArray();
// Footer size includes the 4 bytes for the size itself
int footerSize = footerBytes.length + 4;
output.write(footerBytes);
// Write footer size as big-endian 4 bytes at the end
output.write((footerSize >> 24) & 0xFF);
output.write((footerSize >> 16) & 0xFF);
output.write((footerSize >> 8) & 0xFF);
output.write(footerSize & 0xFF);

return output.toByteArray();
}
}
Loading