FromTimeSpan returns 0 for any time span with a day component

[Scott-Emberson]

FromTimeSpan reads the time span fields at fixed offsets, and takes the day component from timeSpan[0:0], which is always the empty string. Anything carrying a day component parses to zero. Fractional seconds are dropped, and an empty string panics on a slice bound.

pkg/machinepolicies/duration_formatter.go:42 (and the identical copy in pkg/machines/):

days, _ := strconv.ParseInt(timeSpan[0:0], 10, 32)
hours, _ := strconv.ParseInt(timeSpan[2:4], 10, 64)
hours += (days * 24)
minutes, _ := strconv.ParseInt(timeSpan[5:7], 10, 64)
seconds, _ := strconv.ParseInt(timeSpan[8:10], 10, 64)

The fixed offsets also assume a single-digit day, and the server does not pad the day component.

What it returns today

Run against v2.114.1:

FromTimeSpan("00:00:00")       = 0s          (correct)
FromTimeSpan("01:00:00")       = 1h0m0s      (correct)
FromTimeSpan("1.00:00:00")     = 0s          want 24h
FromTimeSpan("02.00:00:00")    = 0s          want 48h
FromTimeSpan("7.12:30:00")     = 12h30m0s    want 180h30m
FromTimeSpan("37500.00:00:00") = 50h0m0s     want 900000h
FromTimeSpan("00:00:00.50000") = 0s          want 500ms
FromTimeSpan("")               = panic: slice bounds out of range [:4] with length 0

Only the hh:mm:ss form works. "1.00:00:00" is the health check interval on the default machine policy, so this is on a common path rather than an edge case.

Why it went unnoticed

pkg/machines/duration_formatter_test.go calls FromTimeSpan seven times and logs each result without asserting anything. Every one of its own inputs returns 0s today and the test passes.

Affected

Any time.Duration field read back through FromTimeSpan. In MachinePolicy alone that is ConnectionConnectTimeout, ConnectionRetrySleepInterval, ConnectionRetryTimeLimit, PollingRequestQueueTimeout and PollingRequestMaximumMessageProcessingTimeout, plus MachineHealthCheckPolicy.HealthCheckInterval and MachineCleanupPolicy.DeleteMachinesElapsedTimeSpan. pkg/machinepolicies/ and pkg/machines/ each hold their own copy of the function.

This also blocks terraform-provider-octopusdeploy#225. The Terraform provider reads a health check interval of one day back as 0, and the fix for that issue makes 0 mean "never run health checks". A policy misparsed as 0 would then be written back to the server as Never, turning health checks off on policies nobody touched.

Suggested fix

Split on the separators instead of slicing at fixed offsets. Both uses of . are ambiguous (d.hh:mm:ss against hh:mm:ss.fffffff), so a leading segment is only the day component when what follows still holds a complete hh:mm:ss. Malformed input should return zero rather than panic.

I have this working in both packages with a table test covering the cases above, and I am happy to open a PR.

Environment

go-octopusdeploy v2.114.1, still present on main.

[NickJosevski]

Fixed by #458, merged 2026-08-23.

FromTimeSpan read the day component from timeSpan[0:0] — always the empty string — so every value carrying days parsed to zero. #458 replaced the fixed-offset parsing with separator-based parsing in both pkg/machines and pkg/machinepolicies, which also picks up fractional seconds, negative time spans and unpadded day components, and returns a zero duration instead of panicking on an empty string.

Verified on main today:

input	before	now
1.00:00:00	0s	24h0m0s
7.12:30:00	12h30m0s	180h30m0s
10.00:00:00	0s	240h0m0s
00:00:00.5000000	0s	500ms
-1.02:03:04	-21h56m56s	-26h3m4s
""	panic	0s

Note for anyone landing here from a behaviour change: this alters values consumers already read. Fields that returned 0s now return the true duration, which can surface as a one-off Terraform plan on machine policy timeouts. Nothing that parsed correctly before has changed.

Thanks @Scott-Emberson for the diagnosis and the original patch in #435 — it was right, and #458 arrived at the same fix independently while #435 was blocked on CI. The test cases from #435 are preserved in #479.