Skip to content
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
19 changes: 16 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,9 @@ jobs:
# The gate reads a real archive with otool, so it runs where the iOS
# library is built: in a release, on all three archives, and in the
# Swift Package job below, on the simulator archive of every pull
# request. This drives everything in it that is text handling, with a
# stand-in for otool, on a runner that has none.
# request. This drives everything in it that is text handling, and the
# packaging function the gate sits inside, with stand-ins for otool,
# lipo and xcodebuild, on a runner that has none of them.
- name: Test the iOS deployment-target gate
run: bash scripts/tests/test-ios-min-os.sh

Expand Down Expand Up @@ -821,7 +822,12 @@ jobs:

- uses: Swatinem/rust-cache@v2
with:
shared-key: ci-swift-package
# The deployment target is the podspec's, and cargo does not rebuild
# a Rust crate when IPHONEOS_DEPLOYMENT_TARGET changes. Without the
# hash a lowered target restores objects stamped for the old one, and
# the minimum-OS gate fails on them with no cargo clean to reach. It
# goes in shared-key: rust-cache ignores `key` when shared-key is set.
shared-key: ci-swift-package-${{ hashFiles('bindings/react-native/MeshSdk.podspec') }}

# One simulator slice, for the architecture of this runner. The release
# builds the device slice and both simulator architectures.
Expand Down Expand Up @@ -989,6 +995,13 @@ jobs:
:offline-protocol-android:testDebugUnitTest
:offline-protocol-android:publishAllPublicationsToStagingRepository

# A call to an API newer than minSdk passes every JVM test and throws on
# an older phone. Lint's NewApi check is the only thing that sees the
# class, and the library build runs it alone (see its lint block).
- name: Lint the library for calls newer than minSdk
working-directory: bindings/kotlin
run: gradle --no-daemon --stacktrace -PVERSION_NAME=0.0.0 :offline-protocol-android:lintDebug

# Gradle succeeds on a test task that found no test. The sources are
# read from another directory, so that is one wrong path away.
- name: Check that the suite ran
Expand Down
37 changes: 33 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ jobs:

- uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
with:
shared-key: release-ios
# The deployment target is the podspec's, and cargo does not rebuild
# a Rust crate when IPHONEOS_DEPLOYMENT_TARGET changes. Without the
# hash a lowered target restores objects stamped for the old one, and
# the minimum-OS gate fails on them with no cargo clean to reach. It
# goes in shared-key: rust-cache ignores `key` when shared-key is set.
shared-key: release-ios-${{ hashFiles('bindings/react-native/MeshSdk.podspec') }}

- name: Cache uniffi-bindgen
id: cache-uniffi
Expand Down Expand Up @@ -1112,24 +1117,48 @@ jobs:
set -euo pipefail
PKG="@offline-protocol/mesh-sdk@$VERSION"

# A version that is not there yet prints nothing, or an E404. An npm
# error of any other kind (a DNS failure, a bad .npmrc, a registry
# outage) is not staging, and reading it as staging would spend the
# whole budget and then blame the registry. So each poll keeps what
# npm said, and the failure below prints the last error. Warnings
# (`npm warn`) are not errors and read as staging.
ERR="$(mktemp)"
LAST_ERROR=""
visible=false
for attempt in $(seq 1 "$VISIBLE_POLLS"); do
if [ "$(npm view "$PKG" version 2>/dev/null || true)" = "$VERSION" ]; then
SEEN="$(npm view "$PKG" version 2>"$ERR" || true)"
if [ "$SEEN" = "$VERSION" ]; then
visible=true
echo "$PKG is readable on the registry (poll $attempt of $VISIBLE_POLLS)."
break
fi
echo "$PKG not readable yet, probably still staging (poll $attempt of $VISIBLE_POLLS)."
if ! grep -qE '^npm (ERR!|error)' "$ERR" || grep -q 'E404' "$ERR"; then
# A missing version after an error means the error passed.
LAST_ERROR=""
echo "$PKG not readable yet, probably still staging (poll $attempt of $VISIBLE_POLLS)."
else
# One line: a workflow annotation ends at the first newline.
# `%` would start an escape in a workflow command.
LAST_ERROR="$(grep -E '^npm (ERR!|error)' "$ERR" | tail -n 5 | tr '\n' ' ' | sed 's/%/%25/g')"
echo "npm view failed, not a staging answer (poll $attempt of $VISIBLE_POLLS): $LAST_ERROR"
fi
sleep "$POLL_SECONDS"
done

if [ "$visible" != true ]; then
echo "::error title=Published version not visible::$PKG was accepted by the \
if [ -n "$LAST_ERROR" ]; then
echo "::error title=npm view failed::$PKG could not be read back for \
$((VISIBLE_POLLS * POLL_SECONDS)) seconds, and npm reported an error rather than \
a missing version, so this may not be staging at all. Last error: $LAST_ERROR"
else
echo "::error title=Published version not visible::$PKG was accepted by the \
publish step but is still not readable after $((VISIBLE_POLLS * POLL_SECONDS)) \
seconds. The registry stages large uploads and can take this long; check \
'npm view $PKG dist.attestations' by hand before doing anything else. A re-run \
is safe (the publish step skips a version that is already there) but only \
waits again."
fi
exit 1
fi

Expand Down
28 changes: 21 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ archived by series under [docs/changelog/](docs/changelog/); see the
record proves the key; a damaged check over an empty store fails with
`FileStoreError::KeyCheckUnverifiable`, naming the file to remove. The
sealed store never deletes a record on a read; a damaged one is reported
as `CorruptedData` naming its file, and one the listing skips is logged,
once per store, as a warning naming its file. Each store holds an
as `CorruptedData` naming its file, and one the listing skips is logged
as a warning naming its file, once until that file is written or removed.
A record that cannot be read is reported as a read failure, never as one
sealed under another key, and never counts towards refusing the store key. Each store holds an
exclusive lock on its directory while open, so a second engine over the
same directories fails with `FileStoreError::InUse` instead of silently
diverging the MLS state. `close()` on either store releases its directory
Expand Down Expand Up @@ -120,8 +122,9 @@ archived by series under [docs/changelog/](docs/changelog/); see the
failed nor a first write that failed can disarm it); a state root that
holds records and belongs with another MLS store (an identity is in
place, but not the one that wrote the state), decided by the pairing ids
the two stores' first open wrote, and for a state root never bound by
whether its sealed records open under this MLS store's record key; an
the two stores' first open wrote, and for a state root never bound, or
bound to another id, by whether its sealed records open under this MLS
store's record key, which only this identity's do; an
account directory that cannot be read, which is never taken for empty;
and roots that are one directory
or one inside the other, compared as spelled before anything is created
Expand Down Expand Up @@ -171,15 +174,26 @@ archived by series under [docs/changelog/](docs/changelog/); see the

### Fixed

- **The storage conformance suite no longer deletes a merging backend's
records.** `runStorageConformance` cleaned up its probe records by listing
a probe key type and deleting what it listed, before any check had run.
On a backend that merges key types, or lists them by the tail of the name,
that listing names real records too, and the cleanup deleted them. The
suite now checks key-type isolation first, with point writes and point
deletes only, and a failure ends the run there with that one check
reported. A run that passes it still has thirteen checks, under the same
names.
- **Relay timestamps parse on Android 7.** The Android bridge parsed them
with `java.time.Instant`, which exists from Android 8 (API 26), while the
SDK supports Android 7 (API 24). On Android 7 the call threw
`NoClassDefFoundError`, which the surrounding `catch (e: Exception)` does
not catch, for a legacy relay message and an ISO-8601 last-seen timestamp.
An application that enables core library desugaring was not affected. The
timestamp is now parsed without `java.time`, with the answers `Instant`
gave, and a Rust guard refuses a new `java.time` call in the bridge. Not
run on an Android 7 device.
timestamp is now parsed without `java.time`, as RFC 3339, and gives the
answers `Instant` gave for every timestamp a relay sends. At the edges it
is stricter than `Instant`: hour 24, a fraction with no digits, lowercase
`t` or `z` and a leap second are refused. A Rust guard refuses a new
`java.time` call in the bridge. Not run on an Android 7 device.
- **An opted-in mesh wake with no wake service stops at once.** A sticky
restart that wakes JavaScript started the wake service by name, and
`startService` returns null rather than throwing when no such service is
Expand Down
13 changes: 13 additions & 0 deletions bindings/kotlin/offline-protocol-android/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,19 @@ android {
withSourcesJar()
}
}

// A call to an API newer than minSdk compiles, passes every unit test on
// the JVM, and throws NoClassDefFoundError on an older phone, which a
// `catch (e: Exception)` does not catch. java.time on Android 7 was one.
// Lint's NewApi check is the class of that defect, so it runs alone and
// fails the build: the other checks report findings this library has not
// decided on. lint.xml leaves out the generated bindings, which guard
// their one such call behind a Class.forName.
lint {
checkOnly += "NewApi"
abortOnError = true
lintConfig = file("lint.xml")
}
}

// A source set is a list of directories and cannot leave a file out, so the
Expand Down
11 changes: 11 additions & 0 deletions bindings/kotlin/offline-protocol-android/lint.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
The generated UniFFI bindings are regenerated, never edited, and guard
their one newer API (java.lang.ref.Cleaner) behind a Class.forName check
that lint cannot follow. Every hand-written source is still checked.
-->
<lint>
<issue id="NewApi">
<ignore path="**/uniffi/**" />
</issue>
</lint>
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.app.NotificationChannel
import android.app.NotificationManager
import android.app.PendingIntent
import android.app.Service
import android.content.ComponentName
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager
Expand Down Expand Up @@ -191,6 +192,14 @@ class MeshForegroundService : Service() {
/** The armed watchdog, or null when no wake is outstanding. */
private var wakeWatchdog: Runnable? = null

/**
* How [dispatchWake] starts the wake service: [startService], and a test
* seam only. `startService` answers null when it starts nothing, and
* Robolectric's never does, so the null branch below is reachable from a
* test only through here.
*/
internal var startWakeService: (Intent) -> ComponentName? = { startService(it) }

inner class LocalBinder : Binder() {
fun getService(): MeshForegroundService = this@MeshForegroundService
}
Expand Down Expand Up @@ -396,7 +405,7 @@ class MeshForegroundService : Service() {
"declared in this application; stopping instead",
)
false
} else if (startService(intent) == null) {
} else if (startWakeService(intent) == null) {
Log.w(TAG, "The mesh wake service did not start; stopping instead")
false
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ package com.offlineprotocol
* API 24 or 25 the call throws `NoClassDefFoundError`, which is an `Error`
* and passes through a `catch (e: Exception)`. Desugaring would have to be
* switched on by every application, not by this library, so the parse is
* written out here and behaves the same on every API level. It answers what
* `Instant.parse` answered for every input the relay sends.
* written out here and behaves the same on every API level.
*
* Stricter than the Swift twin, as `Instant` was: Foundation's formatter
* rolls 2023-02-29 over into March and accepts 24:00 and a `+0530` offset,
* and this refuses all three.
* It reads RFC 3339 internet date-time and answers what `Instant.parse`
* answered for every timestamp the relay sends. It is not `Instant` at the
* edges. It refuses hour 24 and a `.` with no digits after it, which RFC 3339
* refuses too, and lowercase `t` or `z` and a leap second `:60`, which RFC
* 3339 allows. `Instant.parse` accepted all four. No relay sends any of them.
*
* Stricter than the Swift twin: Foundation's formatter rolls 2023-02-29 over
* into March and accepts 24:00 and a `+0530` offset, and this refuses all
* three.
*/
object RelayTimestamps {
/**
Expand All @@ -30,9 +35,13 @@ object RelayTimestamps {
* RFC 3339 internet date-time, what the Swift twin's
* `ISO8601DateFormatter` accepts with `.withInternetDateTime`: a `T`, an
* optional fraction, and `Z` or a numeric offset.
*
* `[0-9]`, never `\d`: on Android `\d` is ICU's and matches every Unicode
* decimal digit, so `٢٠٢٤` would match here and then parse as 2024. The
* JVM the unit tests run on reads `\d` as ASCII and cannot tell.
*/
private val INTERNET_DATE_TIME = Regex(
"""(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.(\d{1,9}))?(Z|([+-])(\d{2}):(\d{2}))"""
"""([0-9]{4})-([0-9]{2})-([0-9]{2})T([0-9]{2}):([0-9]{2}):([0-9]{2})(?:\.([0-9]{1,9}))?(Z|([+-])([0-9]{2}):([0-9]{2}))"""
)

fun parseToMsOrNull(timestampStr: String): Long? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,21 @@ class WifiDirectManager(
if (connected && hasRequiredPermissions()) {
wifiP2pManager?.requestConnectionInfo(channel) { info ->
info?.let {
val previousOwner = groupOwnerAddress
isGroupOwner = it.isGroupOwner
groupOwnerAddress = it.groupOwnerAddress?.hostAddress
// A new connection starts its redial ladder from the
// bottom. The delay doubles on each failed dial and was
// reset only by a proved stream or stop(), so a new group's
// first failed dial waited out the old group's backoff.
// Group owners on Android nearly always take the same
// address, so this compares against the null the
// disconnect branch below leaves, not one owner with
// another: it resets on every connection after a
// disconnect, the same group rejoined included.
if (groupOwnerAddress != previousOwner) {
reconnectDelayMs.set(RECONNECT_INITIAL_DELAY_MS)
}

emitDiagnostic("info", "Connection info", mapOf(
"isGroupOwner" to isGroupOwner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,23 @@ class MeshForegroundServiceTest {
assertTrue(shadowOf(service).isStoppedBySelf)
}

@Test
fun `a wake service that resolves but does not start stops at once`() {
// The service is declared, so it resolves, and the platform still
// starts nothing: `startService` answers null. Robolectric's never
// does, so the start goes through the seam.
optInToWake()
declareWakeService()

val service = createService()
service.startWakeService = { null }
val result = service.onStartCommand(null, 0, 1)

assertEquals(Service.START_NOT_STICKY, result)
assertTrue(shadowOf(service).isForegroundStopped)
assertTrue(shadowOf(service).isStoppedBySelf)
}

@Test
fun `the wake watchdog stops the keep-alive when no host arrives`() {
optInToWake()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,38 @@ class RelayTimestampsTest {
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00+0530"))
}

/** Each field's own bound, one at a time: every other field is valid. */
@Test
fun refusesAFieldOutOfItsRange() {
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:60:00Z"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:60Z"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00+24:00"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00+05:60"))
// The largest value of each is still read.
assertEquals(
1704153599000L,
RelayTimestamps.parseToMsOrNull("2024-01-01T23:59:59Z")
)
assertEquals(
1704067200000L - (23 * 3600L + 59 * 60L) * 1000L,
RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00+23:59")
)
}

/**
* Where this parts from `Instant.parse`, which accepted each of these. RFC 3339
* refuses hour 24 and an empty fraction, and allows lowercase `t`/`z` and
* a leap second; this refuses all of them, and no relay sends any.
*/
@Test
fun refusesWhatInstantAcceptedAndNoRelaySends() {
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T24:00:00Z"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00.Z"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01t00:00:00Z"))
assertNull(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00z"))
assertNull(RelayTimestamps.parseToMsOrNull("2016-12-31T23:59:60Z"))
}

@Test
fun epochSecondsAreScaledToMilliseconds() {
// ~2024-07 as epoch seconds; without the heuristic this would render
Expand Down
14 changes: 11 additions & 3 deletions bindings/react-native/ios/WifiDirectManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -274,16 +274,24 @@ public class WifiDirectManager: NSObject, TransportManager {
advertiser?.stopAdvertisingPeer()
advertiser = nil

// The session is forgotten first. The state-change and data callbacks
// check `self.session === session` on linkQueue, so from here a .connected
// that was already queued finds nothing to attach to. Cleared after
// endAll(), such a callback created a link in the just-emptied table,
// start() never clears it, and a remote that kept its MCPeerID met a
// stale refused link after a restart: no preamble, a refusal at its
// deadline, and one wasted round before the disconnect cleared it.
let old = session
session = nil

// Report every proved peer lost while the core still holds its link,
// on linkQueue so no delivery lands after a loss report. The session's
// own .notConnected callbacks then find nothing left to report.
onLinkQueueSync {
peers.endAll()
}

// Disconnect session
session?.disconnect()
session = nil
old?.disconnect()

// Notify protocol
try? protocolInstance.wifiDirectStatusChanged(isConnected: false)
Expand Down
2 changes: 1 addition & 1 deletion bindings/react-native/ios/tests/RelayTimestampsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ final class RelayTimestampsTests: XCTestCase {

// No zone: a local time, which names no instant. The Kotlin twin also
// refuses an impossible date, 24:00 and an offset without a colon, which
// Foundation's formatter accepts. That difference predates both suites.
// Foundation's formatter accepts. No relay sends any of the three.
func testRefusesATimeWithNoZone() {
XCTAssertNil(RelayTimestamps.parseToMsOrNull("2024-01-01T00:00:00"))
}
Expand Down
Loading
Loading