Bug 509341 - Empathy silently drops incoming messages when certain clients (e.g. blackberry) are in the contact list
Summary: Empathy silently drops incoming messages when certain clients (e.g. blackberr...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: loudmouth
Version: 12
Hardware: All
OS: All
low
urgent
Target Milestone: ---
Assignee: Brian Pepple
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-07-02 10:43 UTC by Felipe Contreras
Modified: 2010-06-03 17:53 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-06-03 14:27:52 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Felipe Contreras 2009-07-02 10:43:39 UTC
Description of problem:
Unable to receive messages.

Version-Release number of selected component (if applicable):
loudmouth-1.4.3-4

How reproducible:
100%

Steps to Reproduce:
1. Add a blackberry contact to your list
2. Re-login
3. Receive a message
  
Actual results:
You don't actually see the message

Expected results:
See the message

Additional info:
There's already an upstream ticket and a patch that fixes the issue:
http://loudmouth.lighthouseapp.com/projects/17276/tickets/39-parser-stops-parser-on-certain-stanzas

Please pick the patch ASAP.

Comment 1 Felipe Contreras 2009-09-20 09:19:53 UTC
What is so difficult about picking a patch and making a release?

Comment 2 Kevin Fenzi 2009-10-17 21:35:44 UTC
Hey Brian. 

If you don't have time, I'd be happy to commit and build this fix for you... looks like an easy one.

Comment 3 Felipe Contreras 2009-10-25 12:25:21 UTC
Looks like Brian doesn't have time. Can we get this fixed?

Comment 4 Kevin Fenzi 2009-10-28 02:15:10 UTC
I talked with Brain the other day. He's going to try and look at this soon, but is very concerned with making changes in stable releases of this package as it's used by a ton of others. 

So, hopefully the patch will be pushed to rawhide pretty soon, and only after some more stress testing to the stable releases. It might help if upstream applied the patch/can note that it's correct and doesn't cause any regressions.

Comment 5 Brian Pepple 2009-11-16 01:53:01 UTC
Pushes to Rawhide. loudmouth-1.4.3-7.fc13

Comment 6 Felipe Contreras 2009-11-17 13:16:36 UTC
(In reply to comment #5)
> Pushes to Rawhide. loudmouth-1.4.3-7.fc13  

Will this be available in F11? It's quite a big issue.

Comment 7 Felipe Contreras 2009-11-17 13:17:21 UTC
(In reply to comment #4)
> So, hopefully the patch will be pushed to rawhide pretty soon, and only after
> some more stress testing to the stable releases. It might help if upstream
> applied the patch/can note that it's correct and doesn't cause any regressions.  

It seems the upstream maintainers have disappeared.

Comment 8 Felipe Contreras 2009-11-30 14:02:31 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Pushes to Rawhide. loudmouth-1.4.3-7.fc13  
> Will this be available in F11? It's quite a big issue.  

Or F12?

By the way, it seems nobody is maintaining loudmouth (last commit was on April) and people are already working on a fork (which contains the fix):
http://github.com/mcabber/loudmouth

Comment 9 Felipe Contreras 2009-12-21 01:41:49 UTC
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Pushes to Rawhide. loudmouth-1.4.3-7.fc13  
> > Will this be available in F11? It's quite a big issue.  
> 
> Or F12?

No reply. Does it mean it's scheduled only for fc13?

Comment 10 Brian Pepple 2009-12-21 01:48:45 UTC
(In reply to comment #9)
> No reply. Does it mean it's scheduled only for fc13?  

Yes, I'm *extremely* leery of pushing this to our stable releases.

Comment 11 Felipe Contreras 2009-12-21 10:43:57 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > No reply. Does it mean it's scheduled only for fc13?  
> 
> Yes, I'm *extremely* leery of pushing this to our stable releases.  

Why?

1) The project is now dead, nobody is maintaining it:

You can see there's no activity:
http://loudmouth.lighthouseapp.com/projects/17276-libloudmouth/overview

And the current maintainer said he is not working on it any more:
http://loudmouth.lighthouseapp.com/projects/17276/tickets/39-parser-stops-parser-on-certain-stanzas

2) There's a fork on github started precisely because of this, and the fork has the patch:

http://github.com/mcabber/loudmouth/commits/master

3) The patch is good

It's on debian testing[1], ubuntu stable[2], and on the Nokia N900. It was written by the Telepathy developers, and they actively pushed for it, I was even asked personally to push it on Fedora.

I can explain to you the patch, but it's obvious that it's good and can't introduce regressions.

If you are not convinced I can gather a bunch of knowledgeable people on the matter and send you a mail personally explaining why this is safe.

[1] http://packages.debian.org/source/testing/loudmouth
[2] https://launchpad.net/ubuntu/+source/loudmouth

Comment 12 Felipe Contreras 2010-02-10 04:49:27 UTC
Hello? How is "reliably not working" better than "working"?

Comment 13 Felipe Contreras 2010-03-06 10:56:28 UTC
So I guess it's Ok for Empathy users to get all their incoming messages silently dropped.

Comment 14 Brian Pepple 2010-06-03 14:27:52 UTC
Fixed in F13 & above. This isn't a critical/security bug so it won't be backported to F12.

Comment 15 Felipe Contreras 2010-06-03 14:41:22 UTC
Pfft. FTR. This was never fixed, libloudmouth was dropped in F13... at least for telepathy-gabble (which I guess was the only user).

Also FTR, anyone with any ability reading code would have seen that this patch is completely harmless. Not to mention the fact that other important distros are shipping it.

Comment 16 Lubomir Rintel 2010-06-03 16:17:54 UTC
(In reply to comment #15)
> Also FTR, anyone with any ability reading code would have seen that this patch
> is completely harmless.

Please prove :)

Even if the patch was harmless (it probably is), the bandwidth is not free, nor is the disk space, nor is the cpu time of users. If it does not seem important enough to you, imagine an update for every tiny patch that fixes someone's problem.

This is why Fedora does releases -- to accumulate such things and release it at once. Users can voluntarily opt-in to run newer releases with features and fixes they need, and you have exactly that choice now.

Comment 17 Felipe Contreras 2010-06-03 17:53:07 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > Also FTR, anyone with any ability reading code would have seen that this patch
> > is completely harmless.
> 
> Please prove :)

Can't you read the aforementioned patch?
http://loudmouth.lighthouseapp.com/projects/17276/tickets/39-parser-stops-parser-on-certain-stanzas

I think it's self-explanatory, but here I go:

Obviously, nobody thought of the case when if (!m)... an error is printed, and the function returns. So, the message is never popped from the queue, and next messages keep piling up, essentially blocking the parser *forever*.

Great, so the whole telepathy CM is just piling messages, and there's not even an error message, or hint of any problem, so the user thinks everything is fine, except that nothing happens: nobody logs in, nobody logs out, nobody changes state, nobody sends any message.

The fix is that was supposed to happen in the first place: log the error, drop the bogus message from the queue, and continue. There's *nothing* that can go wrong if you apply the fix because the infrastructure is supposed to do that anyway.

See?

before:
---
if (!m) {
	g_warning("bad message");
	return;
}

callback(p, m, p->data);
message_unref(m);

unref(p->cur_root);
p->cur_node = p->cur_root = NULL;
---

after:
---
if (!m) {
	g_warning("bad message");
} else {
	callback(p, m, p->data);
	unref(m);
}

unref(p->cur_root);
p->cur_node = p->cur_root = NULL;

unref(p->cur_root);
---

> Even if the patch was harmless (it probably is), the bandwidth is not free, nor
> is the disk space, nor is the cpu time of users. If it does not seem important
> enough to you, imagine an update for every tiny patch that fixes someone's
> problem.

The patch is tiny, which is good, the problem is not, it's important, and is *CRITICAL*. I renders telepathy unusable when you have a contact with a bogus client, like blackberry... I'm pretty sure I'm not the only one which such contacts. Even worst, it does this *silently* so the users would not even notice they have been loosing messages.

> This is why Fedora does releases -- to accumulate such things and release it at
> once. Users can voluntarily opt-in to run newer releases with features and
> fixes they need, and you have exactly that choice now.    

Yes, but if you find a critical bug, which as a simple patch, extremely obvious that can't possibly break anything, that has *already* been tested throughly by developers and users, that is already shipped in mass-product devices like the Nokia N900, shipped in multiple distributions. There's really not much to think.

Not applying it is extremely stupid, and I feel sorry for F12 users who might suddenly wonder why nobody is talking to them (just because one of his friends decided buy a blackberry).


Note You need to log in before you can comment on or make changes to this bug.