Reject attributes the SDKs would clamp

[krassx]

Summary

• AttributePolicy accepts only string, bool, and finite numbers inside the signed 64-bit range, and caps strings at 1024 UTF-16 units.
• Rejection text names the key and the bound, not the value. An empty user identifier clears on both set and get.

Test plan

• DOTNET_ROLL_FORWARD=LatestMajor dotnet test Tests~/WrapperPolicy/Bugsee.WrapperPolicy.Tests.csproj --filter AttributePolicyTests

Made with Cursor

[cursor]

Stale comment

Deep code review — PR #7 (Reject attributes the SDKs would clamp)

Policy-only PR (same shape as the other wrapper-policy PRs): AttributePolicy / UserIdentifierPolicy plus isolated net8 tests. I compared it to the existing Unity bridges and to the React Native Phase 5 rules this is clearly porting (validateAttributeValue, isWithinBundleNumberLimit, normalizeIdentifier).

Findings

P2 Medium — sbyte / ushort / uint / ulong / decimal are rejected as non-numbers

• Location: Runtime/WrapperPolicy/AttributePolicy.cs:32-36
• Problem: The switch only matches byte/short/int/long (plus float/double). C# pattern matching does not treat a boxed uint as int. Those values hit default and return "must be string, bool, or number" even when they are finite and inside the signed 64-bit range.
• Impact: The method's stated job is the RN/Android bundle domain (string | bool | finite number with |x| < 2^63). Once this is wired to Bugsee.SetAttribute, Unity callers passing uint scores, ulong IDs, or sbyte will be refused. iOS already serializes all of those as JSON numbers in IOSBridge.ToJsonValue.
• Scenario: AttributePolicy.Evaluate("wave", 3u) → Accepted = false. Same for (sbyte)(-1), (ushort)1, 1UL, 1m.
• Fix: Accept every primitive numeric TypeCode. Keep the existing 2^63 exclusive bound for float/double. For ulong / decimal, apply the same bound (ulong > long.MaxValue, |decimal| >= 2^63); a bare case ulong _: would accept ulong.MaxValue, which Android's JSON writer would clamp.

P3 Low — NaN / ±Infinity reuse the type-error message

• Location: Runtime/WrapperPolicy/AttributePolicy.cs:48-49 and 55-56
• Problem: Non-finite numbers are rejected with "must be string, bool, or number", the same text as DateTime. RN splits this: non-finite vs |x| >= 2^63 vs wrong type.
• Impact: A computed double.NaN is a number; the message tells the caller the type is wrong.
• Scenario: Evaluate("n", double.NaN) and Evaluate("n", DateTime.UtcNow) produce the same error class of text. The test asserts only Accepted.
• Fix: Use a finite/bound message for NaN/∞ (and keep not echoing the value).

P3 Low — attribute names are not validated

• Location: Runtime/WrapperPolicy/AttributePolicy.cs:22
• Problem: key is only interpolated into errors. Evaluate("", "x") and Evaluate(null, true) return Accepted = true. RN validateAttributeName rejects empty/non-string names; IosReport.SetAttribute already no-ops on empty names.
• Impact: A later facade that only consults Evaluate will still send empty/null keys to JNI / setAttribute:withValue:.
• Scenario: Bugsee.SetAttribute("", "ok") after wiring.
• Fix: Reject null/empty keys in Evaluate (or add a sibling name check and call both from the facade).

No P0/P1. I did not run dotnet test — dotnet is not installed in this environment.

Overall

1. Overall risk: Medium
2. Merge recommendation: Request changes — fix the numeric TypeCode gap before this becomes the SetAttribute gate.
3. Most important to fix: Treat sbyte/ushort/uint/ulong/decimal as numbers, with the 2^63 exclusive bound on the unsigned/decimal paths.
4. What looks good: |x| >= 2^63 matches RN/isWithinBundleNumberLimit (Android longValue() clamp). 1024 is UTF-16 units, matching Android. Rejection text names the key and the decimal bound 9223372036854775808 without echoing the value (avoids double.ToString rounding). Empty user identifier → null on set and get matches both native SDKs and RN normalizeIdentifier.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep code review — PR #7 (47612b7)

Re-review after Accept all primitive numeric attribute types and validate names. Compared again to RN Phase 5 (validateAttributeValue / isWithinBundleNumberLimit / normalizeIdentifier), IOSBridge.ToJsonValue, and AndroidJavaConverters.Box. Policy is still unwired from Bugsee.SetAttribute (same shape as the other wrapper-policy PRs).

Prior findings

• P2 numeric TypeCodes — fully_addressed. Type.GetTypeCode covers sbyte/ushort/uint/ulong/decimal. ulong and decimal use the exclusive 2^63 bound (ulong.MaxValue and ±9223372036854775808m reject; (ulong)long.MaxValue and (decimal)long.MaxValue accept). CLR unbox of a ulong-backed enum to ulong succeeds, so that cast does not throw.
• P3 NaN/±Infinity message — fully_addressed. Non-finite floats take MagnitudeError (same combined finite/bound text RN uses). Test asserts the decimal bound, not the type-error string.
• P3 empty names — fully_addressed. string.IsNullOrEmpty(key) rejects "" and null.

Findings

No remaining P0–P3. I ran DOTNET_ROLL_FORWARD=LatestMajor dotnet test Tests~/WrapperPolicy/Bugsee.WrapperPolicy.Tests.csproj --filter AttributePolicyTests: 6/6 passed. Extra probes (unsigned bounds, ±2^63 decimal, long.MinValue as long vs double, UTF-16 emoji length, char, enums) matched the intended domain.

Residual only (not blocking): no Unity .meta for Runtime/WrapperPolicy/ on this branch (sibling policy PRs commit them). These types are static and unreferenced by GUID, so import churn is the only effect. When wiring the facade, AndroidJavaConverters.Box still stringifies uint/ulong/sbyte/decimal — a pre-existing bridge gap, not this policy.

Overall

1. Overall risk: Low
2. Merge recommendation: Approve
3. Most important to fix: Nothing in this PR. Next step is calling Evaluate / ForSet/ForGet from the facade (and tightening Android Box so accepted numeric TypeCodes stay numbers).
4. What looks good: Intersection domain matches RN/Android (string / bool / finite number with |x| < 2^63, 1024 UTF-16 units). Rejection text names the key and the exact decimal bound 9223372036854775808 without echoing the value. Empty user identifier → null on set and get matches both native SDKs and normalizeIdentifier.

  

_{Sent by Cursor Automation: Bugsee code review}