Closed Bug 1244859 Opened 9 years ago Closed 9 years ago

Remove trailing slash from telemetry ping url

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox45 fixed, firefox46 fixed, firefox47 fixed, fennec45+)

RESOLVED FIXED
Firefox 47
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed
fennec 45+ ---

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(1 file)

13:12 <@mreid> mcomella: you should remove that trailing slash though 13:12 <@mreid> (it doesn't appear to affect anything on the server, it's just not to-spec) 13:13 <mcomella> mreid: So `.../buildId` as opposed to `.../buildId/` ? 13:13 <@mreid> mcomella: yep
It was not to-spec (though it doesn't appear to have an effect on how the server reads the data). Review commit: https://reviewboard.mozilla.org/r/33097/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33097/
Attachment #8714470 - Flags: review?(mark.finkle)
Only bothering to get a review so we can uplift later.
Comment on attachment 8714470 [details] MozReview Request: Bug 1244859 - Remove trailing slash in telemetry urls. r=mfinkle r=mfinkle https://reviewboard.mozilla.org/r/33097/#review29931
Attachment #8714470 - Flags: review?(mark.finkle) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Let's make sure this gets uplifted with bug 1205835.
Blocks: 1205835
No longer depends on: 1205835
(It's not strictly necessary for bug 1205835 though, just an optimization)
No longer blocks: 1205835
Depends on: 1205835
Comment on attachment 8714470 [details] MozReview Request: Bug 1244859 - Remove trailing slash in telemetry urls. r=mfinkle r=mfinkle This is a micro-optimization on the implementation in bug 1205835, which must be uplifted first. Approval Request Comment [Feature/regressing bug #]: bug 1205835. [User impact if declined]: Users send an extra byte (the trailing url slash) when requesting a url. Otherwise, no impact. It's not technically correct according to the server spec but it handles it gracefull. [Describe test coverage new/current, TreeHerder]: Tested locally. [Risks and why]: Extremely low – we remove a character from a url string. Worst case, we upload to the wrong url and break uploads. [String/UUID change made/needed]: None
Attachment #8714470 - Flags: approval-mozilla-beta?
Attachment #8714470 - Flags: approval-mozilla-aurora?
Comment on attachment 8714470 [details] MozReview Request: Bug 1244859 - Remove trailing slash in telemetry urls. r=mfinkle r=mfinkle Minor change, taking it. Should be in 45 beta 4.
Attachment #8714470 - Flags: approval-mozilla-beta?
Attachment #8714470 - Flags: approval-mozilla-beta+
Attachment #8714470 - Flags: approval-mozilla-aurora?
Attachment #8714470 - Flags: approval-mozilla-aurora+
This can't be uplifted without bug 1205835 as it changes files introduced in it. As such, it looks like these uplifted patches created the files in the state they build on top of, when they should be changing one line. Can this get backed out until bug 1205835 lands?
Flags: needinfo?(sledru)
Flags: needinfo?(cbook)
This (and probably a bunch of the related uplift requests) needs to be rebased to work around the lack of bug 1107811 on the release branches.
Flags: needinfo?(michael.l.comella)
I had to back this out on both branches so that everything else could land cleanly and in the correct order before finally relanding it. The backouts are in the following commits: https://hg.mozilla.org/releases/mozilla-aurora/rev/b923a7b09924 https://hg.mozilla.org/releases/mozilla-beta/rev/9b566531611e
Flags: needinfo?(michael.l.comella)
Wes did it. Michael, do we want to land this again? Thanks
Flags: needinfo?(sledru) → needinfo?(michael.l.comella)
Flags: needinfo?(cbook)
(In reply to Sylvestre Ledru [:sylvestre] from comment #17) > Wes did it. > Michael, do we want to land this again? Thanks This is already landed – the commits are posted out of order. The backout in comment 16 occurred before the relanding in comment 15 & comment 14.
Flags: needinfo?(michael.l.comella)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: