341340 - microsummary service should identify itself when downloading pages

Status: RESOLVED FIXED

(Firefox Graveyard :: Microsummaries, defect, P2)

Description

[Myk Melez [:myk] [@mykmelez]]

When the microsummary service downloads a page to summarize it, it should identify itself to the server so the server can adjust pageview stats, block problematic generators, and generally be able to tell the difference between a microsummary service-initiated request and a user-initiated request.

One option is to modify the user-agent for the requests.  Another is to add an extra HTTP header.  Are there others, and which is best?

Comment 1

[Darin Fisher]

We use "X-moz: prefetch" for link-based prefetching, so it might make sense to extend that header.  "X-moz: microsummary" perhaps?

Comment 2

[Mike Connor [:mconnor]]

We really need this.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #1)
> We use "X-moz: prefetch" for link-based prefetching, so it might make sense to
> extend that header.  "X-moz: microsummary" perhaps?

Makes sense to me.

Comment 4

[Simon Bünzli]

Once bug 339543 is fixed, this should be a matter of adding one line (request.setHeader("X-moz", "microsummary")) to MicrosummaryResource.load -- reducing Microsummary SWAGs by about 1.4 days. Right?

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #4)
> Once bug 339543 is fixed, this should be a matter of adding one line
> (request.setHeader("X-moz", "microsummary")) to MicrosummaryResource.load --
> reducing Microsummary SWAGs by about 1.4 days. Right?

Right.  Adding dependency and reducing swag (but to 0.25d rather than 0.1d, as I'm calculating swag in 0.25d increments).

Comment 6

[Simon Bünzli]

Attachment: one-line fix

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 229344 [details] [diff] [review]
one-line fix

>+    request.setRequestHeader("X-moz", "microsummary");

Works great, just one minor issue: the prefetch service capitalizes the m in moz when adding this header, i.e. it specifies X-Moz rather than X-moz:

http://lxr.mozilla.org/mozilla/source/uriloader/prefetch/nsPrefetchService.cpp#244
http://lxr.mozilla.org/mozilla/source/uriloader/prefetch/nsPrefetchService.cpp#341

We should be consistent with it.

Comment 8

[Simon Bünzli]

Attachment: one-line fix without nit

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 229737 [details] [diff] [review]
one-line fix without nit

Perfect.  Just to make sure this is what we want, let's have Darin sr this.

Comment 10

[Darin Fisher]

Comment on attachment 229737 [details] [diff] [review]
one-line fix without nit

be sure to document this someplace where website admins can easily find it, sr=darin

Comment 11

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js.in 1.26

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 229737 [details] [diff] [review]
one-line fix without nit

Notes for drivers considering this approval request:

This is a trivial fix with a very low risk of regression.  It has been on the trunk since Wednesday morning.

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 229737 [details] [diff] [review]
one-line fix without nit

a=mconnor on behalf of drivers for checkin to the 1.8 branch