Skip to content

Commit

Permalink
No longer use stat in timezone database implementations (#385)
Browse files Browse the repository at this point in the history
<https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api>
states that Apple will require the apps to declare all usages of
`stat`, as it can be used to check the file timestamps.
  • Loading branch information
dkhalanskyjb committed Apr 19, 2024
1 parent 9de96c0 commit ce30554
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 60 deletions.
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

1 comment on commit ce30554

@hgourvest
Copy link

Choose a reason for hiding this comment

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

If this doesn't disrupt your schedule, could you produce an RC3 version?
as this commit fixes a bug on Fedora that I just discovered in the RC2 version:
"kotlin.IllegalStateException: Failed to get the system timezone"

Please sign in to comment.