983350 - Send explicit user agent for FxA and token server requests

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P1)

Description

[Katie Parlante]

As per: https://bugzilla.mozilla.org/show_bug.cgi?id=968990#c4, send the same user agent string used for sync requests.

We're currently hacking around this by keeping the user agent string around in elasticsearch/kibana and querying on HttpClient for Android segmentations in kibana.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: github PR

Comment 2

[Mike Trinkala [:trink]]

Those user agents are not RFC 2068 compliant (http://www.faqs.org/rfcs/rfc2068.html)
The product cannot have a space and the version should be slash delimited.

User-Agent 	=	"User-Agent" ":" 1*( product | comment )
product 	=	token ["/" product-version ]
product-version =	token
comment 	=	"(" *( ctext | comment ) ")"
ctext 	        =	<any TEXT excluding "(" and ")">
token 	        =	1*<any CHAR except CTLs or tspecials>
tspecials 	=	"(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/" | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Trinkala [:trink] from comment #2)
> Those user agents are not RFC 2068 compliant
> (http://www.faqs.org/rfcs/rfc2068.html)
> The product cannot have a space and the version should be slash delimited.
> 
> User-Agent 	=	"User-Agent" ":" 1*( product | comment )
> product 	=	token ["/" product-version ]
> product-version =	token
> comment 	=	"(" *( ctext | comment ) ")"
> ctext 	        =	<any TEXT excluding "(" and ")">
> token 	        =	1*<any CHAR except CTLs or tspecials>
> tspecials 	=	"(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/"
> | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT

Interesting.  I'm not sure we care enough to address this now.  For the record, Sync's User-Agent has had spaces for about 2 years; and Apache's HttpClient lib has used for 
CoreProtocolPNames.USER_AGENT: Apache-HttpClient/ (java 1.5) (on Android) for ever.  So clearly this doesn't cause major issues in the wild.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #3)
> (In reply to Mike Trinkala [:trink] from comment #2)
> > Those user agents are not RFC 2068 compliant
> > (http://www.faqs.org/rfcs/rfc2068.html)
> > The product cannot have a space and the version should be slash delimited.
> > 
> > User-Agent 	=	"User-Agent" ":" 1*( product | comment )
> > product 	=	token ["/" product-version ]
> > product-version =	token
> > comment 	=	"(" *( ctext | comment ) ")"
> > ctext 	        =	<any TEXT excluding "(" and ")">
> > token 	        =	1*<any CHAR except CTLs or tspecials>
> > tspecials 	=	"(" | ")" | "<" | ">" | "@" | "," | ";" | ":" | "\" | <"> | "/"
> > | "[" | "]" | "?" | "=" | "{" | "}" | SP | HT
> 
> Interesting.  I'm not sure we care enough to address this now.  For the
> record, Sync's User-Agent has had spaces for about 2 years; and Apache's
> HttpClient lib has used for 
> CoreProtocolPNames.USER_AGENT: Apache-HttpClient/ (java 1.5) (on Android)
> for ever.  So clearly this doesn't cause major issues in the wild.

Ah, sorry, I was mis-reading.  I see that Apache's UA is fine.

Comment 5

[Nick Alexander :nalexander [he/him]]

Landed with the new User-Agent headers obeying the RFC (at least, I hope so :) and the old left untouched (for existing ops query stability, etc).

https://hg.mozilla.org/integration/fx-team/rev/f5109b04e083

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f5109b04e083

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: f5109b04e083

[Approval Request Comment]
Bug caused by (feature/regressing bug #): initial FxA landing.

User impact if declined: *user* impact: almost none.  Ops/metrics impact: more difficult for FxA metrics to determine which version of Android FxA client is hitting endpoints.  This changes a generic User-Agent like:

Apache-HttpClient/UNAVAILABLE (java 1.5)

to a more informative User-Agent including the Firefox for Android version, like:

Firefox-Android-FxAccounts/ (Nightly 31.0a1)

Testing completed (on m-c, etc.): a few days on Nightly and now Aurora; lots of correct User-Agent headers in logs.

Risk to taking this patch (and alternatives if risky): very low.

String or IDL/UUID changes made by this patch: none.

Comment 8

[Nick Alexander :nalexander [he/him]]

kparlante: this is the ticket tracking uplifting the beta User-Agent string.

Comment 9

[Katie Parlante]

(In reply to Nick Alexander :nalexander from comment #8)
> kparlante: this is the ticket tracking uplifting the beta User-Agent string.

Thanks!

Comment 10

[Mike Trinkala [:trink]]

Katie, a lot of these user agents will parse with no version number according to https://github.com/mozilla/persona/blob/dev/lib/coarse_user_agent_parser.js.  Is that ok?

Comment 11

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/releases/mozilla-beta/rev/250ead6a8c06

Comment 12

[Katie Parlante]

(In reply to Mike Trinkala [:trink] from comment #10)
> Katie, a lot of these user agents will parse with no version number
> according to
> https://github.com/mozilla/persona/blob/dev/lib/coarse_user_agent_parser.js.
> Is that ok?

No, we need the version number from this Android user agent. Being consistent with the persona coarse_user_agent_parser is not particularly important fwiw. I'll file a bug on the puppet repo (unless you want to reopen this bug with a different proposal for the user agent string).

Comment 13

[Mike Trinkala [:trink]]

"Firefox-Android-FxAccounts/ (Nightly 31.0a1)" is a new addition and it is incorrect according to the user agent grammar, ideally it would be fixed at the source instead of tweaking downstream consumers (i.e., https://github.com/mozilla-services/puppet-config/issues/312)

Comment 14

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Trinkala [:trink] from comment #13)
> "Firefox-Android-FxAccounts/ (Nightly 31.0a1)" is a new addition and it is
> incorrect according to the user agent grammar, ideally it would be fixed at
> the source instead of tweaking downstream consumers (i.e.,
> https://github.com/mozilla-services/puppet-config/issues/312)

Tell me what you want and I'll do it.  I made an error parsing the spec once, and was thrown off by what HttpClient does, so just say what you want the formatting to be and I'll make it happen.

Comment 15

[Mike Trinkala [:trink]]

Product/Version is fine
Firefox-Android-FxAccounts/31.0a1 (Nightly)

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Trinkala [:trink] from comment #15)
> Product/Version is fine
> Firefox-Android-FxAccounts/31.0a1 (Nightly)

Thanks for the quick response!  Should land in the next few days, and probably in place by EOW (considering uplifts).

Comment 17

[Nick Alexander :nalexander [he/him]]

Attachment: Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/444

rnewman: rubber-stamp?  My perspective is that this is minimal enough to carry forward the a= and triple-land all the way to beta.  What say you?

Comment 18

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #17)
> Created attachment 8400103 [details] [review]
> Link to Github pull-request:
> https://github.com/mozilla-services/android-sync/pull/444
> 
> rnewman: rubber-stamp?  My perspective is that this is minimal enough to
> carry forward the a= and triple-land all the way to beta.  What say you?

From the pull request:  This replaces

Firefox-Android-FxAccounts/ (Nightly 31.0a1)

with

Firefox-Android-FxAccounts/31.0a1 (Nightly)

kparlante: your stamp of approval?

Comment 19

[Richard Newman [:rnewman]]

Comment on attachment 8400103 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/444

Yes, it's a trivial fix. a=follow-up.

Comment 20

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/d198b1b29ccb
https://hg.mozilla.org/releases/mozilla-aurora/rev/7ef9e9c7ceec
https://hg.mozilla.org/releases/mozilla-beta/rev/238e16079ab2

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d198b1b29ccb

Comment 22

[Katie Parlante]

Approved, thx