tzif handling: Should add_civil_datetimes_to_transitions be handling the 0th local time types index?

[Manishearth]

Hi!

I've been working with tzif in temporal_rs, specifically working on boa-dev/temporal#467, and I wanted to confirm an intuition about the tzif data so I had a look at the jiff code, and I think I found a bug? I'm not 100% sure.

In add_civil_datetimes_to_transitions, jiff calculates the offset before a transition by subtracting 1 from the transition index (saturating to 0 on underflow) and using that transition:

jiff/src/shared/tzif.rs

Lines 608 to 618 in 71ea43d

	let trans = &mut self.transitions;
	for i in 0..trans.timestamps.len() {
	let timestamp = trans.timestamps[i];
	let offset = {
	let type_index = trans.infos[i].type_index;
	self.types[usize::from(type_index)].offset
	};
	let prev_offset = {
	let type_index = trans.infos[i.saturating_sub(1)].type_index;
	self.types[usize::from(type_index)].offset
	};

This means, for the first transition, the prev_offset will be the offset for that transition. The first transition is then always an unambiguous transition from an offset to itself.

I think that's incorrect: the prev_offset in that case should be self.types[0]. Looking at a bunch of tzif objects; none of their transitions reference entry 0 (-10:31:26 ). For example, even the example Pacific/Honolulu entry has no trans type entry with a value of 0. Checking against TZDB, I get the following entry:

# Zone	NAME		STDOFF	RULES	FORMAT	[UNTIL]
Zone Pacific/Honolulu	-10:31:26 -	LMT	1896 Jan 13 12:00
			-10:30	-	HST	1933 Apr 30  2:00
			-10:30	1:00	HDT	1933 May 21 12:00
			-10:30	US	H%sT	1947 Jun  8  2:00
			-10:00	-	HST

which also has a -10:31:26 entry that ends at Jan 13 1896, and has no start date. This doesn't quite match the tzif spec (which has the first entry at Jan 13 1901, I don't know what that's about), but it seems to match the point that the first local time type is the "starting" one before the first transition.

I wish the spec actually mentioned this, it's aggravatingly terse, but that's old IETF specs for you.

Figured i'd file this issue to let you know. There's a good chance I've just missed something here, but worth raising in case it is a real bug. The fix, I think, would be to obtain self.types[0] instead of doing the saturating_sub when the index is 0.

[Manishearth]

Aha, found the spec line

The type corresponding to a transition time specifies local time for
timestamps starting at the given transition time and continuing up
to, but not including, the next transition time. Local time for
timestamps before the first transition is specified by the first time
type (time type 0).

Sounds like my guess was right.