Skip to content

Commit

Permalink
fix: 💥 handle directory entries with length of 0 according to PMTiles…
Browse files Browse the repository at this point in the history
… spec

PMTiles spec states that dir entries must have a length that is greater than 0, therefore this library will handle such cases as errors while reading,  writing and constructing PMTiles archives
  • Loading branch information
DerZade committed May 18, 2024
1 parent 0d5acb4 commit 5105a5e
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 41 deletions.
45 changes: 40 additions & 5 deletions src/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,16 @@ impl Directory {

// read length
for i in 0..num_entries {
entries[i].length = read_varint([_], [reader])?;
let len = read_varint([_], [reader])?;

if len == 0 {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"Length of a directory entry must be greater than 0.",
));
}

entries[i].length = len;
}

// read offset
Expand Down Expand Up @@ -178,6 +187,12 @@ impl Directory {

// write length
for entry in &self.entries {
if entry.length == 0 {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"Length of a directory entry must be greater than 0.",
));
}
write_varint([writer], [entry.length])?;
}

Expand Down Expand Up @@ -295,8 +310,9 @@ impl Directory {
/// * `compression` - Compression to use
///
/// # Errors
/// Will return [`Err`] if `compression` is set to [`Compression::Unknown`] or an I/O
/// error occurred while writing to `output`.
/// Will return [`Err`] if `compression` is set to [`Compression::Unknown`], the
/// directory includes a entry with a length of 0 or an I/O error occurred
/// while writing to `output`.
///
/// # Example
/// ```rust
Expand All @@ -320,8 +336,9 @@ impl Directory {
/// * `compression` - Compression to use
///
/// # Errors
/// Will return [`Err`] if `compression` is set to [`Compression::Unknown`] or an I/O
/// error occurred while writing to `output`.
/// Will return [`Err`] if `compression` is set to [`Compression::Unknown`], the
/// directory includes a entry with a length of 0 or an I/O error occurred
/// while writing to `output`.
///
/// # Example
/// ```rust
Expand Down Expand Up @@ -462,4 +479,22 @@ mod test {

Ok(())
}

#[test]
fn test_to_writer_invalid_entry() {
let mut dir = Directory {
entries: Vec::new(),
};

dir.entries.push(Entry {
length: 0,
offset: 0,
run_length: 0,
tile_id: 0,
});

let mut buf = Vec::<u8>::with_capacity(10);
let mut writer = Cursor::new(&mut buf);
assert!(dir.to_writer(&mut writer, ROOT_DIR_COMPRESSION).is_err());
}
}
8 changes: 6 additions & 2 deletions src/pmtiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,12 @@ impl<R> PMTiles<R> {
/// Note that the data should already be compressed if [`Self::tile_compression`] is set to a value other than [`Compression::None`].
/// The data will **NOT** be compressed automatically.
/// The [`util`-module](crate::util) includes utilities to compress data.
pub fn add_tile(&mut self, tile_id: u64, data: impl Into<Vec<u8>>) {
self.tile_manager.add_tile(tile_id, data);
///
/// # Errors
/// Will return [`Err`] if `data` converts into an empty `Vec`.
///
pub fn add_tile(&mut self, tile_id: u64, data: impl Into<Vec<u8>>) -> Result<()> {
self.tile_manager.add_tile(tile_id, data)
}

/// Removes a tile from this archive.
Expand Down
87 changes: 53 additions & 34 deletions src/tile_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,16 @@ impl<R> TileManager<R> {
}

/// Add tile to writer
pub fn add_tile(&mut self, tile_id: u64, data: impl Into<Vec<u8>>) {
pub fn add_tile(&mut self, tile_id: u64, data: impl Into<Vec<u8>>) -> Result<()> {
let vec: Vec<u8> = data.into();

if vec.is_empty() {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
"A tile must have at least 1 byte of data.",
));
}

// remove tile just to make sure that there
// are no unreachable tiles
self.remove_tile(tile_id);
Expand All @@ -70,6 +77,8 @@ impl<R> TileManager<R> {
self.data_by_hash.insert(hash, vec);

self.ids_by_hash.entry(hash).or_default().insert(tile_id);

Ok(())
}

pub(crate) fn add_offset_tile(&mut self, tile_id: u64, offset: u64, length: u32) {
Expand Down Expand Up @@ -262,7 +271,7 @@ mod test {

let contents = vec![1u8, 3, 3, 7, 4, 2];

manager.add_tile(42, contents.clone());
manager.add_tile(42, contents.clone())?;

let opt = manager.get_tile(42)?;

Expand All @@ -273,48 +282,54 @@ mod test {
}

#[test]
fn test_add_tile() {
fn test_add_tile() -> Result<()> {
let mut manager = TileManager::default();

manager.add_tile(1337, vec![1, 3, 3, 7, 4, 2]);
manager.add_tile(1337, vec![1, 3, 3, 7, 4, 2])?;
assert_eq!(manager.data_by_hash.len(), 1);

manager.add_tile(42, vec![4, 2, 1, 3, 3, 7]);
manager.add_tile(42, vec![4, 2, 1, 3, 3, 7])?;
assert_eq!(manager.data_by_hash.len(), 2);

Ok(())
}

#[test]
fn test_add_tile_dedup() {
fn test_add_tile_dedup() -> Result<()> {
let mut manager = TileManager::default();

let contents = vec![1u8, 3, 3, 7, 4, 2];

manager.add_tile(42, contents.clone());
manager.add_tile(1337, contents);
manager.add_tile(42, contents.clone())?;
manager.add_tile(1337, contents)?;

assert_eq!(manager.data_by_hash.len(), 1);

Ok(())
}

#[test]
fn test_add_tile_update() {
fn test_add_tile_update() -> Result<()> {
let mut manager = TileManager::default();

manager.add_tile(1337, vec![1, 3, 3, 7, 4, 2]);
manager.add_tile(1337, vec![1, 3, 3, 7, 4, 2])?;
assert_eq!(manager.data_by_hash.len(), 1);
assert_eq!(manager.tile_by_id.len(), 1);
assert_eq!(manager.ids_by_hash.len(), 1);

manager.add_tile(1337, vec![4, 2, 1, 3, 3, 7]);
manager.add_tile(1337, vec![4, 2, 1, 3, 3, 7])?;
assert_eq!(manager.data_by_hash.len(), 1);
assert_eq!(manager.tile_by_id.len(), 1);
assert_eq!(manager.ids_by_hash.len(), 1);

Ok(())
}

#[test]
fn test_remove_tile() {
fn test_remove_tile() -> Result<()> {
let mut manager = TileManager::default();

manager.add_tile(42, vec![1u8, 3, 3, 7, 4, 2]);
manager.add_tile(42, vec![1u8, 3, 3, 7, 4, 2])?;

assert_eq!(manager.tile_by_id.len(), 1);
assert_eq!(manager.data_by_hash.len(), 1);
Expand All @@ -325,6 +340,8 @@ mod test {
assert_eq!(manager.tile_by_id.len(), 0);
assert_eq!(manager.data_by_hash.len(), 0);
assert_eq!(manager.ids_by_hash.len(), 0);

Ok(())
}

#[test]
Expand All @@ -337,13 +354,13 @@ mod test {
}

#[test]
fn test_remove_tile_dupe() {
fn test_remove_tile_dupe() -> Result<()> {
let mut manager = TileManager::default();

let contents = vec![1u8, 3, 3, 7, 4, 2];
manager.add_tile(69, contents.clone());
manager.add_tile(42, contents.clone());
manager.add_tile(1337, contents);
manager.add_tile(69, contents.clone())?;
manager.add_tile(42, contents.clone())?;
manager.add_tile(1337, contents)?;

assert_eq!(manager.data_by_hash.len(), 1);

Expand All @@ -358,6 +375,8 @@ mod test {
manager.remove_tile(42);
assert_eq!(manager.data_by_hash.len(), 0);
assert_eq!(manager.ids_by_hash.len(), 0);

Ok(())
}

#[test]
Expand All @@ -368,9 +387,9 @@ mod test {
let tile_42 = vec![42u8, 3, 3, 7, 4, 2];
let tile_1337 = vec![1u8, 3, 3, 7, 4, 2];

manager.add_tile(0, tile_0.clone());
manager.add_tile(42, tile_42.clone());
manager.add_tile(1337, tile_1337.clone());
manager.add_tile(0, tile_0.clone())?;
manager.add_tile(42, tile_42.clone())?;
manager.add_tile(1337, tile_1337.clone())?;

let result = manager.finish()?;
let data = result.data;
Expand All @@ -391,9 +410,9 @@ mod test {

let content = vec![1u8, 3, 3, 7, 4, 2];

manager.add_tile(0, content.clone());
manager.add_tile(1, vec![1]);
manager.add_tile(1337, content.clone());
manager.add_tile(0, content.clone())?;
manager.add_tile(1, vec![1])?;
manager.add_tile(1337, content.clone())?;

let result = manager.finish()?;
let data = result.data;
Expand All @@ -419,8 +438,8 @@ mod test {
manager.add_offset_tile(0, 0, 4);
manager.add_offset_tile(5, 0, 4);
manager.add_offset_tile(10, 4, 4);
manager.add_tile(15, vec![1, 3, 3, 7]);
manager.add_tile(20, vec![1, 3, 3, 7]);
manager.add_tile(15, vec![1, 3, 3, 7])?;
manager.add_tile(20, vec![1, 3, 3, 7])?;

let result = manager.finish()?;
let data = result.data;
Expand Down Expand Up @@ -451,11 +470,11 @@ mod test {

let content = vec![1u8, 3, 3, 7, 4, 2];

manager.add_tile(0, content.clone());
manager.add_tile(1, content.clone());
manager.add_tile(2, content.clone());
manager.add_tile(3, content.clone());
manager.add_tile(4, content);
manager.add_tile(0, content.clone())?;
manager.add_tile(1, content.clone())?;
manager.add_tile(2, content.clone())?;
manager.add_tile(3, content.clone())?;
manager.add_tile(4, content)?;

let result = manager.finish()?;
let directory = result.directory;
Expand All @@ -474,10 +493,10 @@ mod test {
let mut manager = TileManager::default();

// add tiles in random order
manager.add_tile(42, vec![42]);
manager.add_tile(1337, vec![13, 37]);
manager.add_tile(69, vec![69]);
manager.add_tile(1, vec![1]);
manager.add_tile(42, vec![42])?;
manager.add_tile(1337, vec![13, 37])?;
manager.add_tile(69, vec![69])?;
manager.add_tile(1, vec![1])?;

let result = manager.finish()?;
let directory = result.directory;
Expand Down

0 comments on commit 5105a5e

Please sign in to comment.