Skip to content

Commit

Permalink
api: Fix usage of int.parse that implicitly accepts hexadecimal
Browse files Browse the repository at this point in the history
Calling just `int.parse(s)`, without a `radix:` argument, invokes
a special behavior where `int.parse` not only accepts decimal strings
like "42", but also hexadecimal strings like "0x2a".

That's a bit unexpected.  In any case it's definitely not something
we want when interpreting any part of the Zulip API.

Fix the one place we had this in our own code.  There remain two
places it appears in the code generated by `json_serializable`;
mark those with TODO comments.  It'd be nice to fix those too,
but realistically this quirk is unlikely to ever cause a problem,
so it's not worth a lot of effort to resolve.

(Note that this doesn't affect the bulk of places we have an int
in the API types, because most of those are handled by jsonDecode
before the `json_serializable`-generated code ever sees them.
It only affects the keys of `Map<int, …>` structures.)

It looks like there's no existing thread in the `json_serializable`
tracker for this issue.  The most closely related is from where the
handling of `Map<int, …>` types was added in the first place:
  google/json_serializable.dart#434
  • Loading branch information
gnprice authored and chrisbobbe committed Oct 18, 2023
1 parent 0847f10 commit f547771
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 1 deletion.
1 change: 1 addition & 0 deletions lib/api/model/events.dart
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ class UpdateMessageFlagsRemoveEvent extends UpdateMessageFlagsEvent {
String get op => 'remove';

// final bool all; // deprecated, ignore
// TODO(json_serializable): keys use plain `int.parse`, permitting hexadecimal
final Map<int, UpdateMessageFlagsMessageDetail>? messageDetails;

UpdateMessageFlagsRemoveEvent({
Expand Down
3 changes: 3 additions & 0 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,13 @@ class User {
String timezone;
String? avatarUrl; // TODO distinguish null from missing https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20omitted.20vs.2E.20null.20in.20JSON/near/1551759
int avatarVersion;

// null for bots, which don't have custom profile fields.
// If null for a non-bot, equivalent to `{}` (null just written for efficiency.)
// TODO(json_serializable): keys use plain `int.parse`, permitting hexadecimal
@JsonKey(readValue: _readProfileData)
Map<int, ProfileFieldUserData>? profileData;

@JsonKey(readValue: _readIsSystemBot)
bool? isSystemBot; // TODO(server-5)

Expand Down
2 changes: 1 addition & 1 deletion lib/api/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class ApiNarrowMessageId extends ApiNarrowElement {
ApiNarrowMessageId(int operand, {super.negated}) : operand = operand.toString();

factory ApiNarrowMessageId.fromJson(Map<String, dynamic> json) => ApiNarrowMessageId(
int.parse(json['operand'] as String),
int.parse(json['operand'] as String, radix: 10),
negated: json['negated'] as bool? ?? false,
);
}

0 comments on commit f547771

Please sign in to comment.