Skip to content

Send proper User-Agent string in requests #453

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.

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
sirpengi opened this issue Dec 14, 2023 · 2 comments
Closed

Send proper User-Agent string in requests #453

sirpengi opened this issue Dec 14, 2023 · 2 comments

Comments

@sirpengi
Copy link
Contributor

Identified as an issue while working on #440 - the Zulip server will automatically mark messages as read for their authors if the client that sent the request is internally identified as a human-powered client (in other words, the message came from an app of some sort rather than a python script).

See sent_by_human in https://github.com/zulip/zulip/blob/main/zerver/models.py#L3238

The name of the client is pulled from the User-Agent header. An initial PR to close #440 was proposed in #450, but that only added a hardcoded User-Agent string as crafting an appropriate one (to match https://github.com/zulip/zulip-mobile/blob/main/src/utils/userAgent.js ) was fraught inside of lib/api/core.dart. Pulling the necessary information out of package_info_plus there broke over a hundred tests as the infrastructure for calculating those values wasn't set up yet.

I think there are a couple options here:

  1. Continue the work of dynamically crafting a User-Agent at send in lib/api/core.dart, figure out the tests.
  2. Craft the User-Agent at a higher level layer, and send that through into the api (such as how we pass zulipFeatureLevel or realmUrl when creating an ApiConnection).

From a cursory look there doesn't appear to be any other use of sent_by_human with other api endpoints, so perhaps this is something that we only need to send when creating a message?

Also see related links:

Additionally, there is a PR in zulip/zulip#28204 that proposes a read_by_sender flag in the API. We could take advantage of this, but will still need a User-Agent for legacy servers.

@timabbott
Copy link
Member

My recommendation is to use the new API from zulip/zulip#28204 and to send a hacky useragent containing ZulipMobile (but perhaps containing ZulipFlutter in it too) for just the send message API request, only if the server's feature level is below the cutoff for #28204.

@gnprice
Copy link
Member

gnprice commented Dec 19, 2023

This is the same as the existing issue #406, so let's track it there.

I think it's best if lib/api/core.dart takes care of this for itself, without adding any new wrinkles to the API that its callers have to handle. A key difference from zulipFeatureLevel or realmUrl is that the value to use is constant for any given run of the app (the only thing that makes it not just a string literal in the source code is the app's version number). Those other two values vary between Zulip servers, so for them ApiConnection fundamentally does need some information from its caller in order to get the right answer.

@gnprice gnprice closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants