Skip to content

Send hard-coded User-Agent in requests #450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Send hard-coded User-Agent in requests #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c06855e
deps: Upgrade Flutter to latest main, 3.18.0-7.0.pre.55
sirpengi Dec 8, 2023
9bb9d81
core: Send hardcoded User-Agent header in requests
sirpengi Dec 13, 2023
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
8 changes: 8 additions & 0 deletions lib/api/core.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class ApiConnection {
assert(debugLog("${request.method} ${request.url}"));

addAuth(request);
request.headers.addAll(userAgentHeader());

final http.StreamedResponse response;
try {
Expand Down Expand Up @@ -198,6 +199,13 @@ Map<String, String> authHeader({required String email, required String apiKey})
};
}

// TODO(#453): Create real User-Agent string
Map<String, String> userAgentHeader() {
return {
'User-Agent': 'ZulipMobile/?.?.? (Android 14)',
Copy link
Member

Choose a reason for hiding this comment

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

Saying "Android 14" means making up bogus, untruthful, data — we might be on some other Android version, or on iOS — so let's avoid that.

By contrast saying "?.?.?" for the version number is fine, at least as a stopgap, because it's honest about what this code doesn't know.

Copy link
Member

Choose a reason for hiding this comment

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

Let's work in something here to make clear this is actually the Flutter app, not the RN app. E.g., saying "ZulipFlutter".

Tim suggests at #453 (comment) that it's enough for "ZulipMobile" to appear somewhere in the value, not necessarily at the start. Can we add something like "(like ZulipMobile)" at the end instead, and put "ZulipFlutter" at the start?

Or if we do need it at the start, in the venerable tradition of every modern browser claiming to be Mozilla 5.0:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent
then in that same venerable tradition I hope we can at least put the more informative name somewhere later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested with live server and dug through https://github.com/zulip/zulip/blob/main/zerver/lib/user_agent.py . The User-Agent is parsed with this:

pattern = re.compile(
    """^ (?P<name> [^/ ]* [^0-9/(]* )
    (/ (?P<version> [^/ ]* ))?
    ([ /] .*)?
    $""",
    re.X,
)

and the whitelist from https://github.com/zulip/zulip/blob/main/zerver/models/clients.py is:

            sending_client
            in (
                "zulipandroid",
                "zulipios",
                "zulipdesktop",
                "zulipmobile",
                "zulipelectron",
                "zulipterminal",
                "snipe",
                "website",
                "ios",
                "android",
            )
            or "desktop app" in sending_client
            # Since the vast majority of messages are sent by humans
            # in Zulip, treat test suite messages as such.
            or (sending_client == "test suite" and settings.TEST_SUITE)

To trigger a match we could not start the string with "ZulipFlutter". I think given the options we should present as "ZulipMobile" and stick on a "(like ZulipMobile)" at the end.

We could possibly start with "ZulipFlutter; like desktop app" or "ZulipFlutter(like desktop app)" (but NOT "ZulipFlutter (like desktop app)") but I don't like these options.

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's do a string like ZulipMobile/1.2.3 (Android 14) (actually ZulipFlutter). Where instead of "Android 14" we have the actual OS name and version (and we can leave that part out for a first pass), and instead of "1.2.3" we have the actual version (and we can leave that out too for a first pass).

Copy link
Member

Choose a reason for hiding this comment

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

In fact, better yet: once we do have the version number, it'll be from a different sequence from the zulip-mobile version numbers anyway. So rather than saying "ZulipMobile/1.2.3", we can let the version number remain absent at that spot and instead say a string like ZulipMobile (Android 14) (actually ZulipFlutter/1.2.3).

};
}

Map<String, String>? encodeParameters(Map<String, dynamic>? params) {
return params?.map((k, v) =>
MapEntry(k, v is RawParameter ? v.value : jsonEncode(v)));
Expand Down
1 change: 0 additions & 1 deletion lib/widgets/message_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'dart:math';
import 'dart:ui';

import 'package:collection/collection.dart';
import 'package:flutter/material.dart';
Expand Down
1 change: 0 additions & 1 deletion lib/widgets/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'dart:io';
import 'dart:ui';
import 'package:flutter/widgets.dart';

/// A mergeable [TextStyle] with 'Source Code Pro' and platform-aware fallbacks.
Expand Down
1 change: 0 additions & 1 deletion lib/widgets/unread_count_badge.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'dart:ui';

import 'package:flutter/material.dart';

Expand Down
8 changes: 4 additions & 4 deletions pubspec.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,10 +768,10 @@ packages:
dependency: "direct main"
description:
name: path
sha256: "8829d8a55c13fc0e37127c29fedf290c102f4e40ae94ada574091fe0ff96c917"
sha256: "087ce49c3f0dc39180befefc60fdb4acd8f8620e5682fe2476afd0b3688bb4af"
url: "https://pub.dev"
source: hosted
version: "1.8.3"
version: "1.9.0"
path_provider:
dependency: "direct main"
description:
Expand Down Expand Up @@ -1282,5 +1282,5 @@ packages:
source: hosted
version: "3.1.2"
sdks:
dart: ">=3.3.0-162.0.dev <4.0.0"
flutter: ">=3.17.0-16.0.pre.23"
dart: ">=3.3.0-212.0.dev <4.0.0"
flutter: ">=3.18.0-7.0.pre.55"
4 changes: 2 additions & 2 deletions pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ environment:
# that by the time we want to release, these will have become stable.
# TODO: Before general release, switch to stable Flutter and Dart versions,
# or pin exact versions: https://github.com/zulip/zulip-flutter/issues/15
sdk: '>=3.3.0-162.0.dev <4.0.0'
flutter: '>=3.17.0-16.0.pre.23'
sdk: '>=3.3.0-212.0.dev <4.0.0'
flutter: '>=3.18.0-7.0.pre.55'

# Dependencies specify other packages that your package needs in order to work.
# To automatically upgrade your package dependencies to the latest versions
Expand Down
12 changes: 10 additions & 2 deletions test/api/core_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ void main() {
check(connection.lastRequest!).isA<http.Request>()
..method.equals('GET')
..url.asString.equals('${eg.realmUrl.origin}$expectedRelativeUrl')
..headers.deepEquals(authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey))
..headers.deepEquals({
...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey),
...userAgentHeader(),
})
..body.equals('');
});
}
Expand Down Expand Up @@ -53,6 +56,7 @@ void main() {
..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route')
..headers.deepEquals({
...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey),
...userAgentHeader(),
if (expectContentType)
'content-type': 'application/x-www-form-urlencoded; charset=utf-8',
})
Expand Down Expand Up @@ -83,7 +87,10 @@ void main() {
check(connection.lastRequest!).isA<http.MultipartRequest>()
..method.equals('POST')
..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route')
..headers.deepEquals(authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey))
..headers.deepEquals({
...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey),
...userAgentHeader(),
})
..fields.deepEquals({})
..files.single.which(it()
..field.equals('file')
Expand Down Expand Up @@ -115,6 +122,7 @@ void main() {
..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route')
..headers.deepEquals({
...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey),
...userAgentHeader(),
if (expectContentType)
'content-type': 'application/x-www-form-urlencoded; charset=utf-8',
})
Expand Down
2 changes: 0 additions & 2 deletions test/flutter_checks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/// `package:checks`-related extensions for the Flutter framework.
library;

import 'dart:ui';

import 'package:checks/checks.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
Expand Down
1 change: 0 additions & 1 deletion test/widgets/text_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'dart:ui';

import 'package:checks/checks.dart';
import 'package:flutter/material.dart';
Expand Down