Closed
Bug 954890
Opened 10 years ago
Closed 10 years ago
Make use of all WHOWAS 312/314 response pairs
Categories
(Chat Core :: IRC, defect)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: aleth, Assigned: clokep)
References
Details
Attachments
(1 file, 3 obsolete files)
1.57 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1457 at 2012-05-22 00:18:00 UTC *** Some servers (eg mozilla) send multiple responses that seem to be in chronological order, giving what amounts to a recent login history. We currently ignore all but the last of these (i.e. the corresponding whois entries are overwritten).
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1457 at 2012-06-23 13:37:51 UTC *** (In reply to bug 954926 (bio 1493), comment #0) > WHOWAS actually returns a sequence of 312/314 messages, e.g. > > irc: :gravel.mozilla.org 314 adev flo Instantbir moz-87C33FDA.com * :Florian > irc: :gravel.mozilla.org 312 adev flo gravel.mozilla.org :Thu Jun 7 14:50:13 > 2012 > irc: :gravel.mozilla.org 314 adev flo florian moz-87C33FDA.com * :purple > irc: :gravel.mozilla.org 312 adev flo gravel.mozilla.org :Thu Jun 7 12:54:30 > 2012 > irc: :gravel.mozilla.org 314 adev flo Instantbir moz-87C33FDA.com * :Florian > irc: :gravel.mozilla.org 312 adev flo concrete.mozilla.org :Thu Jun 7 12:02:18 > 2012 > irc: :gravel.mozilla.org 369 adev flo :End of WHOWAS > > Currently we only use the final pair (as the previous entries in > whoisinformation get overwritten). This is less informative and actually > incorrect, as they are sorted in descending time order.
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1457 as attmnt 1898 at 2012-09-04 03:14:00 UTC *** This uses the first response we get from WHOWAS, I might have made this a bit more complicated than necessary (perhaps we just shouldn't overwrite the value if it exists already?), but it seemed reasonable. I also debated trying to parse the date, but: 1. Not all servers give it. 2. Parsing dates is a PITA.
Attachment #8353656 -
Flags: review?(bugzilla)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8353656 [details] [diff] [review] Patch *** Original change on bio 1457 attmnt 1898 at 2012-09-04 10:50:18 UTC *** This seems to fail to properly handle the corresponding WHOIS responses.
Attachment #8353656 -
Flags: review?(bugzilla)
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1457 as attmnt 1900 at 2012-09-05 01:09:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353658 -
Flags: review?(bugzilla)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8353656 [details] [diff] [review] Patch *** Original change on bio 1457 attmnt 1898 at 2012-09-05 01:09:02 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353656 -
Attachment is obsolete: true
Reporter | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1457 at 2012-09-05 21:24:09 UTC *** This works, but I am wondering (as you indicate in comment 3) why we are going to be storing all the responses for 312 and not the paired ones for 314 (so the current patch prints the most recent 312 with the oldest 314), and if it's just the mozilla server where the 312/314 pairs arrive in reverse chronological order (so that the assumption this patch is based on is more valid then the existing "use the last arrival" code).
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 1457 at 2012-09-06 00:49:40 UTC *** (In reply to comment #6) > This works, but I am wondering (as you indicate in comment 3) why we are going > to be storing all the responses for 312 and not the paired ones for 314 I don't follow this at all, aren't we storing all the 312 responses and 314 responses? > (so the current patch prints the most recent 312 with the oldest 314), How so? Don't we use the newest one (which is the first received response) for both? I'm not sure how you know what is "newest" for 314, I'm just assuming it goes with the closest 312. > and if it's just > the mozilla server where the 312/314 pairs arrive in reverse chronological > order (so that the assumption this patch is based on is more valid then the > existing "use the last arrival" code). I'm not really sure what this is talking about...
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8353658 [details] [diff] [review] Patch v2 - store all received entries *** Original change on bio 1457 attmnt 1900 at 2012-09-06 10:56:19 UTC *** (In reply to comment #7) > > to be storing all the responses for 312 and not the paired ones for 314 > I don't follow this at all, aren't we storing all the 312 responses and 314 > responses? Uh, right, sorry. I got confused by looking at the diff on bugzilla only. > > and if it's just > > the mozilla server where the 312/314 pairs arrive in reverse chronological > > order (so that the assumption this patch is based on is more valid then the > > existing "use the last arrival" code). > I'm not really sure what this is talking about... The net effect of the patch is that we show the first pair to arrive rather than the last. I was just wondering if we know that this is on average more likely to be the right one. But so far the only server I've seen that even returns more than one pair is mozilla.org so I guess the answer is "yes" ;)
Attachment #8353658 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 1457 at 2012-10-12 10:42:37 UTC *** (In reply to comment #8) > Comment on attachment 8353658 [details] [diff] [review] (bio-attmnt 1900) [details] > Patch v2 > > > > and if it's just > > > the mozilla server where the 312/314 pairs arrive in reverse chronological > > > order (so that the assumption this patch is based on is more valid then the > > > existing "use the last arrival" code). > > I'm not really sure what this is talking about... > The net effect of the patch is that we show the first pair to arrive rather > than the last. I was just wondering if we know that this is on average more > likely to be the right one. But so far the only server I've seen that even > returns more than one pair is mozilla.org so I guess the answer is "yes" ;) So do we think this is fine the way it is...or should we stop storing all the pairs and simply use the last one. But if so, isn't that the same as what we previously had?
Reporter | ||
Comment 11•10 years ago
|
||
*** Original post on bio 1457 at 2012-10-12 16:43:18 UTC *** (In reply to comment #9) We should either take this patch as is or simplify it to just show the first pair (correct on mozilla.org, so far our only use case). Taking it as is might make it easier to make changes later if other server behaviours turn up (or to show all the received info if anyone cares).
Assignee | ||
Comment 12•10 years ago
|
||
*** Original post on bio 1457 as attmnt 1955 at 2012-10-12 18:28:00 UTC *** This will only use the first whois set if one is repeated.
Attachment #8353714 -
Flags: review?(bugzilla)
Assignee | ||
Comment 13•10 years ago
|
||
*** Original post on bio 1457 as attmnt 1956 at 2012-10-12 19:04:00 UTC *** I received some feedback over IRC.
Attachment #8353715 -
Flags: review?(florian)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8353714 [details] [diff] [review] Alternate patch v1 *** Original change on bio 1457 attmnt 1955 at 2012-10-12 19:04:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353714 -
Attachment is obsolete: true
Attachment #8353714 -
Flags: review?(bugzilla)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8353715 [details] [diff] [review] Alternate patch v2 *** Original change on bio 1457 attmnt 1956 at 2012-10-16 19:10:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353715 -
Flags: review?(florian) → review?(bugzilla)
Reporter | ||
Comment 16•10 years ago
|
||
Comment on attachment 8353715 [details] [diff] [review] Alternate patch v2 *** Original change on bio 1457 attmnt 1956 at 2012-10-16 19:15:16 UTC *** as seen on IRC ;)
Attachment #8353715 -
Flags: review?(bugzilla) → review+
Reporter | ||
Comment 17•10 years ago
|
||
Comment on attachment 8353658 [details] [diff] [review] Patch v2 - store all received entries *** Original change on bio 1457 attmnt 1900 at 2012-10-16 19:17:12 UTC *** Tagging this in case it is ever needed again in another context.
Attachment #8353658 -
Attachment description: Patch v2 → Patch v2 - store all received entries
Attachment #8353658 -
Attachment is obsolete: true
Reporter | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 18•10 years ago
|
||
*** Original post on bio 1457 at 2012-10-25 12:06:31 UTC *** (In reply to comment #12) > Created attachment 8353715 [details] [diff] [review] (bio-attmnt 1956) [details] > Alternate patch v2 Committed as http://hg.instantbird.org/instantbird/rev/d30db418fd7c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•