Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer use stat in timezone database implementations #385

Merged
merged 1 commit into from
Apr 19, 2024
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
3 changes: 1 addition & 2 deletions core/androidNative/src/internal/TzdbBionic.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ internal fun TzdbBionic(): TimeZoneDatabase = TzdbBionic(buildMap<String, TzdbBi
Path.fromString("/system/usr/share/zoneinfo/tzdata"), // immutable fallback tzdb
Path.fromString("/apex/com.android.tzdata/etc/tz/tzdata"), // an up-to-date tzdb, may not exist
)) {
if (path.check() == null) continue // the file does not exist
// be careful to only read each file a single time and keep many references to the same ByteArray in memory.
val content = path.readBytes()
val content = path.readBytes() ?: continue // this file does not exist
val header = BionicTzdbHeader.parse(content)
val indexSize = header.data_offset - header.index_offset
check(indexSize % 52 == 0) { "Invalid index size: $indexSize (must be a multiple of 52)" }
Expand Down
21 changes: 15 additions & 6 deletions core/tzdbOnFilesystem/src/internal/TzdbOnFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,28 @@ package kotlinx.datetime.internal

internal class TzdbOnFilesystem(defaultTzdbPath: Path? = null): TimeZoneDatabase {

private val tzdbPath = tzdbPaths(defaultTzdbPath).find {
it.chaseSymlinks().check()?.isDirectory == true
internal val tzdbPath = tzdbPaths(defaultTzdbPath).find { path ->
tabPaths.any { path.containsFile(it) }
} ?: throw IllegalStateException("Could not find the path to the timezone database")

override fun rulesForId(id: String): TimeZoneRules =
readTzFile(tzdbPath.resolve(Path.fromString(id)).readBytes()).toTimeZoneRules()
override fun rulesForId(id: String): TimeZoneRules {
val idAsPath = Path.fromString(id)
require(!idAsPath.isAbsolute) { "Timezone ID '$idAsPath' must not begin with a '/'" }
require(idAsPath.components.none { it == ".." }) { "'$idAsPath' must not contain '..' as a component" }
val file = Path(tzdbPath.isAbsolute, tzdbPath.components + idAsPath.components)
val contents = file.readBytes() ?: throw RuntimeException("File '$file' not found")
return readTzFile(contents).toTimeZoneRules()
}

override fun availableTimeZoneIds(): Set<String> = buildSet {
tzdbPath.traverseDirectory(exclude = tzdbUnneededFiles) { add(it.toString()) }
tzdbPath.tryTraverseDirectory(exclude = tzdbUnneededFiles) { add(it.toString()) }
}

}

// taken from https://github.com/tzinfo/tzinfo/blob/9953fc092424d55deaea2dcdf6279943f3495724/lib/tzinfo/data_sources/zoneinfo_data_source.rb#L354
private val tabPaths = listOf("zone1970.tab", "zone.tab", "tab/zone_sun.tab")

/** The files that sometimes lie in the `zoneinfo` directory but aren't actually time zones. */
private val tzdbUnneededFiles = setOf(
// taken from https://github.com/tzinfo/tzinfo/blob/9953fc092424d55deaea2dcdf6279943f3495724/lib/tzinfo/data_sources/zoneinfo_data_source.rb#L88C29-L97C21
Expand Down Expand Up @@ -51,7 +60,7 @@ internal fun tzdbPaths(defaultTzdbPath: Path?) = sequence {

// taken from https://github.com/HowardHinnant/date/blob/ab37c362e35267d6dee02cb47760f9e9c669d3be/src/tz.cpp#L3951-L3952
internal fun pathToSystemDefault(): Pair<Path, Path>? {
val info = Path(true, listOf("etc", "localtime")).chaseSymlinks()
val info = chaseSymlinks("/etc/localtime") ?: return null
val i = info.components.indexOf("zoneinfo")
if (!info.isAbsolute || i == -1 || i == info.components.size - 1) return null
return Pair(
Expand Down
34 changes: 18 additions & 16 deletions core/tzdbOnFilesystem/src/internal/filesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,37 @@ package kotlinx.datetime.internal
import kotlinx.cinterop.*
import platform.posix.*

internal fun Path.chaseSymlinks(maxDepth: Int = 100): Path {
var realPath = this
var depth = maxDepth
while (true) {
realPath = realPath.readLink() ?: break
if (depth-- == 0) throw RuntimeException("Too many levels of symbolic links")
}
return realPath
internal fun chaseSymlinks(name: String): Path? = memScoped {
val buffer = allocArray<ByteVar>(PATH_MAX)
realpath(name, buffer)?.let { Path.fromString(it.toKString()) }
}

internal fun Path.traverseDirectory(exclude: Set<String> = emptySet(), stripLeadingComponents: Int = this.components.size, actionOnFile: (Path) -> Unit) {
val handler = opendir(this.toString()) ?: return
internal fun Path.containsFile(file: String): Boolean = access("$this/$file", F_OK) == 0

internal fun Path.tryTraverseDirectory(
exclude: Set<String> = emptySet(),
stripLeadingComponents: Int = this.components.size,
maxDepth: Int = 100,
actionOnFile: (Path) -> Unit
): Boolean {
if (maxDepth <= 0) throw IllegalStateException("Max depth reached: $this")
val handler = opendir(this.toString()) ?: return false
try {
while (true) {
val entry = readdir(handler) ?: break
val name = entry.pointed.d_name.toKString()
if (name == "." || name == "..") continue
if (name in exclude) continue
val path = Path(isAbsolute, components + name)
val info = path.check() ?: continue // skip broken symlinks
if (info.isDirectory) {
if (!info.isSymlink) {
path.traverseDirectory(exclude, stripLeadingComponents, actionOnFile)
}
} else {
val isDirectory = path.tryTraverseDirectory(
exclude, stripLeadingComponents, maxDepth = maxDepth - 1, actionOnFile
)
if (!isDirectory) {
actionOnFile(Path(false, path.components.drop(stripLeadingComponents)))
}
}
} finally {
closedir(handler)
}
return true
}
8 changes: 3 additions & 5 deletions core/tzdbOnFilesystem/test/TimeZoneRulesCompleteTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@ class TimeZoneRulesCompleteTest {
@OptIn(ExperimentalEncodingApi::class)
@Test
fun iterateOverAllTimezones() {
val root = Path.fromString("/usr/share/zoneinfo")
val tzdb = TzdbOnFilesystem(root)
val tzdb = TzdbOnFilesystem()
for (id in tzdb.availableTimeZoneIds()) {
val file = root.resolve(Path.fromString(id))
val rules = tzdb.rulesForId(id)
runUnixCommand("env LOCALE=C zdump -V $file").windowed(size = 2, step = 2).forEach { (line1, line2) ->
runUnixCommand("env LOCALE=C zdump -V ${tzdb.tzdbPath}/$id").windowed(size = 2, step = 2).forEach { (line1, line2) ->
val beforeTransition = parseZdumpLine(line1)
val afterTransition = parseZdumpLine(line2)
try {
Expand Down Expand Up @@ -51,7 +49,7 @@ class TimeZoneRulesCompleteTest {
} catch (e: Throwable) {
println(beforeTransition)
println(afterTransition)
println(Base64.encode(file.readBytes()))
println(Base64.encode(Path.fromString("${tzdb.tzdbPath}/$id").readBytes()!!))
throw e
}
}
Expand Down
33 changes: 2 additions & 31 deletions core/tzfile/src/internal/filesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,6 @@ import kotlinx.cinterop.*
import platform.posix.*

internal class Path(val isAbsolute: Boolean, val components: List<String>) {
fun check(): PathInfo? = memScoped {
val stat = alloc<stat>()
val err = stat(this@Path.toString(), stat.ptr)
if (err != 0) return null
object : PathInfo {
override val isDirectory: Boolean = stat.st_mode.toInt() and S_IFMT == S_IFDIR // `inode(7)`, S_ISDIR
override val isSymlink: Boolean = stat.st_mode.toInt() and S_IFMT == S_IFLNK // `inode(7)`, S_ISLNK
}
}

fun readLink(): Path? = memScoped {
val buffer = allocArray<ByteVar>(PATH_MAX)
val err = readlink(this@Path.toString(), buffer, PATH_MAX.convert<size_t>())
if (err == (-1).convert<ssize_t>()) return null
buffer[err] = 0
fromString(buffer.toKString())
}

fun resolve(other: Path): Path = when {
other.isAbsolute -> other
else -> Path(isAbsolute, components + other.components)
}

override fun toString(): String = buildString {
if (isAbsolute) append("/")
if (components.isNotEmpty()) {
Expand All @@ -53,14 +30,8 @@ internal class Path(val isAbsolute: Boolean, val components: List<String>) {
}
}

// `stat(2)` lists the other available fields
internal interface PathInfo {
val isDirectory: Boolean
val isSymlink: Boolean
}

internal fun Path.readBytes(): ByteArray {
val handler = fopen(this.toString(), "rb") ?: throw RuntimeException("Cannot open file $this")
internal fun Path.readBytes(): ByteArray? {
val handler = fopen(this.toString(), "rb") ?: return null
try {
var err = fseek(handler, 0, SEEK_END)
if (err == -1) throw RuntimeException("Cannot jump to the end of $this: $errnoString")
Expand Down