Closed
Bug 954532
Opened 10 years ago
Closed 10 years ago
Authorization loop if lastMessageId is empty for a Twitter account
Categories
(Chat Core :: Twitter, defect)
Chat Core
Twitter
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: clokep, Assigned: florian)
Details
Attachments
(1 file, 1 obsolete file)
2.04 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1098 at 2011-10-21 00:28:00 UTC *** I found this when testing stuff on my Twitter account: I cleared the lastMessageId (so it became an empty string). Our twitter code then throws an error and requests reauthorization. If you try to authorize, it just asks again and loops. I'm not sure if this could happen without users messing with their profile, but we should attempt to handle a bad value in there (if possible).
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1098 at 2011-10-21 00:32:09 UTC *** The error is: Error: 400 - {"error":"Error in 'since_id' parameter: invalid id.","request":"\/1\/statuses\/mentions.json?include_entities=1&count=200&since_id="} Source File: file:///C:/Users/clokep/instantbird2/objdir-ib-release/mozilla/dist/bin/components/twitter.js Line: 558 Source Code: twitter
Reporter | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1098 at 2011-11-09 00:46:55 UTC *** As flo said on IRC: well, from a quick read of the summary, it seems there are actually 2 different issues: 1. The bad handling of that preference value (probably trivial to fix) 2. The authorization loop; how come we detect a 401 - unauthorized error if there's a syntax error? it's possible the OAuth code misbehaves for a parameter with an empty value, in which case we should fix it it's also possible the twitter server sends us an inappropriate reply, in which case we can't do much I'm working on this.
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1098 as attmnt 981 at 2011-11-09 00:55:00 UTC *** This is the "easy" part of the fix, which simply verifies that the ID we've loaded is made up of digits. While testing this, the lastMessageId preference didn't seem to update between restarting my Instantbird, perhaps fallout from flo's big rewrite that was just checked in?
Attachment #8352722 -
Flags: review?(florian)
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1098 as attmnt 1073 at 2011-12-11 23:04:00 UTC *** Taking over, with an updated/version of attachment 8352722 [details] [diff] [review] (bio-attmnt 981): - fixed the regexp - added warning when we ignore the value of a pref - fixed the broken detection of the 401 HTTP status.
Attachment #8352815 -
Flags: review?(clokep)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8352722 [details] [diff] [review] Verify the lastMessageId preference *** Original change on bio 1098 attmnt 981 at 2011-12-11 23:04:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352722 -
Attachment is obsolete: true
Attachment #8352722 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: clokep → florian
Assignee | ||
Comment 6•10 years ago
|
||
*** Original post on bio 1098 at 2011-12-11 23:05:22 UTC *** (In reply to comment #4) > - fixed the broken detection of the 401 HTTP status. Filed https://bugzilla.mozilla.org/show_bug.cgi?id=709622 to get a JS strict warning added for this trivial code error.
Reporter | ||
Comment 7•10 years ago
|
||
Comment on attachment 8352815 [details] [diff] [review] Patch v2 *** Original change on bio 1098 attmnt 1073 at 2011-12-11 23:07:52 UTC *** Looks good, thanks for investigating the second part of this (and fixing my regex!). :)
Attachment #8352815 -
Flags: review?(clokep) → review+
Reporter | ||
Comment 8•10 years ago
|
||
*** Original post on bio 1098 at 2011-12-12 01:09:37 UTC *** Checked in as http://hg.instantbird.org/instantbird/rev/5a9c9760fb59
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•