Closed Bug 1024811 Opened 5 years ago Closed 5 years ago

hawkclient will fail if no response headers

Categories

(Firefox :: Firefox Accounts, defect)

defect
Not set
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 33
Iteration:
34.3
Tracking Status
firefox30 --- wontfix
firefox31 + fixed
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: markh, Assigned: markh)

Details

Attachments

(1 file)

When we try and find retry related headers, the code assumes response.headers is non null - see http://hg.mozilla.org/mozilla-central/file/95cc116fac5a/services/common/hawkclient.js#l238

Note that the current tip of that file does *not* have the problem - it was fixed independently in revision fe1432e7b895.

Also worrying is the fact that the exception caused by this is not reported anywhere - we had to deduce this from other log entries - see bug 1006360 comment 61 and 62 - but I'll open yet another bug for that as trunk *is* still affected by that, just not by the "null headers" problem here.

(Note I'm not 100% sure about the status flags below - the fix is in revision fe1432e7b895 - so I *think* that means 32 and 33 are already fixed.)
Flags: firefox-backlog+
This patch checks we have headers before attempting to use them, and also fixes a use of the log.  This patch is against beta - the same patch landed on central in revision fe1432e7b895 as part of bug 988469 and is already in Aurora.
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Attachment #8439637 - Flags: review?(rnewman)
Marco, please add this to the current iteration
Flags: needinfo?(mmucci)
Whiteboard: p=2 [qa-]
Added to Iteration 33.1
Flags: needinfo?(mmucci)
Whiteboard: p=2 [qa-] → p=2 s=33.1 [qa-]
Comment on attachment 8439637 [details] [diff] [review]
0001-Bug-1024811-hawkclient-now-checks-if-headers-are-non.patch

Review of attachment 8439637 [details] [diff] [review]:
-----------------------------------------------------------------

Looks sane to me.
Attachment #8439637 - Flags: review?(rnewman) → review+
Comment on attachment 8439637 [details] [diff] [review]
0001-Bug-1024811-hawkclient-now-checks-if-headers-are-non.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): FxA sync
User impact if declined: Sync may work incorrectly on some network errors related to auth.
Testing completed (on m-c, etc.): This same patch has already landed on m-c and Aurora via bug 988469 - this patch is to add the same code to beta.
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None

As above, this needs to land on beta only - central and aurora already have the fix.
Attachment #8439637 - Flags: approval-mozilla-beta?
Attachment #8439637 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Iteration: --- → 33.2
Points: --- → 2
QA Whiteboard: [qa-]
Whiteboard: p=2 s=33.1 [qa-]
Depends on: 1029652
No longer depends on: 1029652
Iteration: 33.2 → ---
Landed on 31, which is the only place it needed to, so fixed.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Iteration: --- → 34.3
QA Whiteboard: [qa-]
Flags: qe-verify-
Product: Core → Firefox
Target Milestone: mozilla33 → Firefox 33
You need to log in before you can comment on or make changes to this bug.