Skip to content

Commit 6973085

Browse files
Fix ABTesting ValidateRunningExperiments runtime drift (#115)
* Fix ABTesting ValidateRunningExperiments runtime drift * Strengthen ABTesting runtime drift regression
1 parent f77b1c5 commit 6973085

4 files changed

Lines changed: 100 additions & 2 deletions

File tree

docs/firebase-runtime-failure-backlog.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ This backlog tracks binding drifts that are concrete candidates for the runtime-
66

77
| Case id | Target/member | Audit finding reference | Expected runtime failure mechanism | Proof status | Fix PR | Merged commit |
88
| --- | --- | --- | --- | --- | --- | --- |
9-
| `abtesting-activateexperiment` | `Firebase.ABTesting.ExperimentController.ActivateExperiment` | `output/firebase-binding-audit/report.json` -> `ABTesting` -> `ActivateExperiment` (`signature-drift`) | The old binding exports `activateExperiment:forServiceOrigin:` as `NSObject` instead of `ABTExperimentPayload`, so callers can send an arbitrary object and native ABTesting immediately raises `ObjCRuntime.ObjCException` when it invokes `ABTExperimentPayload` selectors on that object. | Proved locally on the unfixed binding. Evidence artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-activateexperiment-failure.json`. Regression artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-activateexperiment-success.json`. | - | - |
9+
| `abtesting-validaterunningexperiments` | `Firebase.ABTesting.ExperimentController.ValidateRunningExperiments` | `output/firebase-binding-audit/report.json` -> `ABTesting` -> `ValidateRunningExperiments` (`signature-drift`) | The old binding exports `validateRunningExperimentsForServiceOrigin:runningExperimentPayloads:` as `NSObject[]` instead of `ABTExperimentPayload[]`, so callers can send arbitrary objects and native ABTesting immediately raises `ObjCRuntime.ObjCException` when it invokes `ABTExperimentPayload` selectors on those objects. | Proved locally on the unfixed binding. Evidence artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-validaterunningexperiments-failure.json`. Regression artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-validaterunningexperiments-success.json`. | - | - |
1010

1111
## Investigating
1212

@@ -16,5 +16,6 @@ No additional runtime-failure candidates are currently promoted.
1616

1717
| Case id | Target/member | Audit finding reference | Runtime failure mechanism | Proof status | Fix PR | Merged commit |
1818
| --- | --- | --- | --- | --- | --- | --- |
19+
| `abtesting-activateexperiment` | `Firebase.ABTesting.ExperimentController.ActivateExperiment` | `output/firebase-binding-audit/report.json` -> `ABTesting` -> `ActivateExperiment` (`signature-drift`) | The old binding exported `activateExperiment:forServiceOrigin:` as `NSObject` instead of `ABTExperimentPayload`, so callers could send an arbitrary object and native ABTesting immediately raised `ObjCRuntime.ObjCException` when it invoked `ABTExperimentPayload` selectors on that object. | Proved and fixed. Evidence artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-activateexperiment-failure.json`. Regression artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-abtesting-activateexperiment-success.json`. | [#114](https://github.com/AdamEssenmacher/GoogleApisForiOSComponents/pull/114) | `f77b1c57eff15cf6c1b5ca0df47d8195b46f0f40` |
1920
| `cloudfunctions-usefunctionsemulatororigin` | `Firebase.CloudFunctions.CloudFunctions.UseFunctionsEmulatorOrigin` | Manual runtime candidate from the current binding surface and native `FirebaseFunctions` Swift header | The binding exported `useFunctionsEmulatorOrigin:` even though the current framework only exposes `useEmulatorWithHost:port:`. Calling the stale selector raised `ObjCRuntime.ObjCException` with an unrecognized-selector native exception. | Proved and fixed. Evidence artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-cloudfunctions-usefunctionsemulatororigin-failure.json`. Regression artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-cloudfunctions-usefunctionsemulatororigin-success.json`. | [#113](https://github.com/AdamEssenmacher/GoogleApisForiOSComponents/pull/113) | `b6a6c82c74e0cf3645edb9dc5519d4a628f7ed36` |
2021
| `cloudfirestore-getquerynamed` | `Firebase.CloudFirestore.Firestore.GetQueryNamed` | `output/firebase-binding-audit/report.json` -> `CloudFirestore` -> `GetQueryNamed` (`signature-drift`) | The old binding exported `getQueryNamed:completion:` as `NSInputStream` instead of `NSString`, so native Firestore received `__NSCFInputStream` and threw an Objective-C exception when it treated the argument like a string. | Proved and fixed. Evidence artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-cloudfirestore-getquerynamed-failure.json`. Regression artifact: `tests/E2E/Firebase.Foundation/artifacts/firebase-foundation-result-cloudfirestore-getquerynamed-success.json`. | [#112](https://github.com/AdamEssenmacher/GoogleApisForiOSComponents/pull/112) | `3077a538a892de0db3350f0a459bf8620729f4db` |

source/Firebase/ABTesting/ApiDefinition.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface ExperimentController {
2727

2828
// -(void)validateRunningExperimentsForServiceOrigin:(NSString * _Nonnull)origin runningExperimentPayloads:(NSArray<ABTExperimentPayload *> * _Nonnull)payloads;
2929
[Export ("validateRunningExperimentsForServiceOrigin:runningExperimentPayloads:")]
30-
void ValidateRunningExperiments (string origin, NSObject [] payloads);
30+
void ValidateRunningExperiments (string origin, ExperimentPayload [] payloads);
3131

3232
// -(void)activateExperiment:(ABTExperimentPayload * _Nonnull)experimentPayload forServiceOrigin:(NSString * _Nonnull)origin;
3333
[Export ("activateExperiment:forServiceOrigin:")]

tests/E2E/Firebase.Foundation/FirebaseFoundationE2E/FirebaseRuntimeDriftCases.cs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
using ObjCRuntime;
77
#endif
88

9+
#if ENABLE_RUNTIME_DRIFT_CASE_ABTESTING_VALIDATERUNNINGEXPERIMENTS
10+
using Firebase.ABTesting;
11+
using Foundation;
12+
using ObjCRuntime;
13+
#endif
14+
915
#if ENABLE_RUNTIME_DRIFT_CASE_CLOUDFIRESTORE_GETQUERYNAMED
1016
using Firebase.CloudFirestore;
1117
using Foundation;
@@ -126,6 +132,86 @@ void OnMarshalObjectiveCException(object? sender, MarshalObjectiveCExceptionEven
126132
}
127133
#endif
128134

135+
#if ENABLE_RUNTIME_DRIFT_CASE_ABTESTING_VALIDATERUNNINGEXPERIMENTS
136+
static Task<string> VerifyABTestingValidateRunningExperimentsAsync()
137+
{
138+
const string selector = "validateRunningExperimentsForServiceOrigin:runningExperimentPayloads:";
139+
140+
var controller = ExperimentController.SharedInstance;
141+
if (controller is null)
142+
{
143+
throw new InvalidOperationException("Firebase.ABTesting.ExperimentController.SharedInstance returned null after App.Configure().");
144+
}
145+
146+
var signature = typeof(ExperimentController).GetMethod(
147+
nameof(ExperimentController.ValidateRunningExperiments),
148+
BindingFlags.Instance | BindingFlags.Public,
149+
binder: null,
150+
types: new[] { typeof(string), typeof(ExperimentPayload[]) },
151+
modifiers: null);
152+
if (signature is null)
153+
{
154+
throw new InvalidOperationException(
155+
$"Expected managed API '{nameof(ExperimentController.ValidateRunningExperiments)}(string, {typeof(ExperimentPayload[]).FullName})' was not found.");
156+
}
157+
158+
var parameters = signature.GetParameters();
159+
if (parameters.Length != 2 || parameters[1].ParameterType != typeof(ExperimentPayload[]))
160+
{
161+
throw new InvalidOperationException(
162+
$"Managed signature regression: expected payload parameter type '{typeof(ExperimentPayload[]).FullName}', observed '{parameters.ElementAtOrDefault(1)?.ParameterType.FullName ?? "<missing>"}'.");
163+
}
164+
165+
var payloads = Array.Empty<ExperimentPayload>();
166+
var origin = "codex";
167+
NSException? marshaledException = null;
168+
MarshalObjectiveCExceptionMode? marshaledExceptionMode = null;
169+
170+
void OnMarshalObjectiveCException(object? sender, MarshalObjectiveCExceptionEventArgs args)
171+
{
172+
marshaledException ??= args.Exception;
173+
marshaledExceptionMode ??= args.ExceptionMode;
174+
}
175+
176+
Runtime.MarshalObjectiveCException += OnMarshalObjectiveCException;
177+
try
178+
{
179+
try
180+
{
181+
controller.ValidateRunningExperiments(origin, payloads);
182+
}
183+
catch (ObjCException ex)
184+
{
185+
throw new InvalidOperationException(
186+
$"Selector '{selector}' should not throw after the binding fix, but observed {ex.GetType().FullName}. " +
187+
$"Managed payload array type: {payloads.GetType().FullName}. Payload count: {payloads.Length}. " +
188+
$"Origin argument type: {origin.GetType().FullName}. " +
189+
$"NSException.Name: {FormatDetail(marshaledException?.Name?.ToString())}. " +
190+
$"NSException.Reason: {FormatDetail(marshaledException?.Reason)}. " +
191+
$"Marshal mode: {FormatDetail(marshaledExceptionMode?.ToString())}.",
192+
ex);
193+
}
194+
195+
if (marshaledException is not null)
196+
{
197+
throw new InvalidOperationException(
198+
$"Selector '{selector}' completed, but Runtime.MarshalObjectiveCException captured unexpected NSException.Name '{marshaledException.Name}'. " +
199+
$"Reason: {FormatDetail(marshaledException.Reason)}. Marshal mode: {FormatDetail(marshaledExceptionMode?.ToString())}.");
200+
}
201+
202+
return Task.FromResult(
203+
$"Selector '{selector}' completed without ObjC exception after the binding fix. " +
204+
$"Managed signature payload type: {parameters[1].ParameterType.FullName}. " +
205+
$"Managed payload array type: {payloads.GetType().FullName}. Payload count: {payloads.Length}. " +
206+
$"Origin argument type: {origin.GetType().FullName}.");
207+
}
208+
finally
209+
{
210+
Runtime.MarshalObjectiveCException -= OnMarshalObjectiveCException;
211+
}
212+
}
213+
#endif
214+
129215
#if ENABLE_RUNTIME_DRIFT_CASE_CLOUDFIRESTORE_GETQUERYNAMED
130216
static async Task<string> VerifyCloudFirestoreGetQueryNamedAsync()
131217
{

tests/E2E/Firebase.Foundation/runtime-drift-cases.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@
1111
}
1212
]
1313
},
14+
{
15+
"id": "abtesting-validaterunningexperiments",
16+
"method": "VerifyABTestingValidateRunningExperimentsAsync",
17+
"bindingPackage": "AdamE.Firebase.iOS.ABTesting",
18+
"packages": [
19+
{
20+
"id": "AdamE.Firebase.iOS.ABTesting",
21+
"version": "12.6.0"
22+
}
23+
]
24+
},
1425
{
1526
"id": "cloudfirestore-getquerynamed",
1627
"method": "VerifyCloudFirestoreGetQueryNamedAsync",

0 commit comments

Comments
 (0)