nautilus crashed with SIGSEGV in strlen()

[Mathieu Marquer]

Binary package hint: nautilus

Steps to reproduce this bug :
* Install nautilus-open-terminal
* Connect nautilus to a SSH server using Raccourcis (Shortcuts ?) -> Se connecter à un serveur (connect to a server ?).
* Right click in the nautilus window displaying the content of the server, select "open to a terminal"
--> Nautilus crashes.

ProblemType: Crash
Architecture: amd64
CrashCounter: 1
DistroRelease: Ubuntu 9.04
ExecutablePath: /usr/bin/nautilus
NonfreeKernelModules: nvidia
Package: nautilus 1:2.25.91-0ubuntu1 [modified: usr/bin/nautilus usr/bin/nautilus-file-management-properties usr/bin/nautilus-autorun-software usr/bin/nautilus-connect-server]
ProcCmdline: nautilus
ProcEnviron:
 LANG=fr_FR.UTF-8
 SHELL=/bin/bash
Signal: 11
SourcePackage: nautilus
StacktraceTop:
 strlen () from /lib/libc.so.6
 strdup () from /lib/libc.so.6
 ?? ()
 g_closure_invoke ()
 ?? () from /usr/lib/libgobject-2.0.so.0
Title: nautilus crashed with SIGSEGV in strlen()
Uname: Linux 2.6.28-8-generic x86_64
UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare src

[Apport retracing service (apport)]

StacktraceTop:strlen () from /lib/libc.so.6
strdup () from /lib/libc.so.6
open_terminal_callback (item=0x2688b20,
IA__g_closure_invoke (closure=0x31265c0,
signal_emit_unlocked_R (node=0x1bf8020, detail=0,

[Pedro Villavicencio (pedro)]

confirmed as per duplicates.

[shemgp (shemgp)]

Here's a possible debdiff. It works for me, though I'm not sure if this is how to patch a patch in a debdiff.

[shemgp (shemgp)]

Oops, forgot the bug # in the other one.

There's a test build in PPA too:
https://launchpad.net/~shemgp/+archive/ppa

[nanog (sorenimpey)]

The patched deb from shemgb fixes problem for 386 and x64.

[Tim Besard (maleadt)]

Hi shemgp, and thanks for your effort writing this patch. It doesn't solve the issue for me though, the terminal now starts, but closes immediately, without crashing nautilus though. Valgrind reveals no illegal operations, it seems like a triggered assertion.
I used the deb at your ppa (amd64).

[Maxim Levitsky (maximlevitsky)]

sad that I didn't look at your fix, because I found and fixed same bug, but here it works too.

@maleadt, when I expiremented with the buggy function I have also seen same behavior.
Could you post the url of your ssh share (You can replace username, and any other sensitive names with something else:

for example:

sftp://<email address hidden>/folder1/folder1/folder3

Just keep all control marks intact (eg '.', '/', ...)

I suspect the parsing code still be buggy.

Also, do you usually enter password in the poped window, or you use public keys?

[Tim Besard (maleadt)]

user/pass authentication, memorized by gnome-keyring. The share its address: sftp://<email address hidden>:12345/folder
It might be the custom port switch which kills the parsing?

[Maxim Levitsky (maximlevitsky)]

Could very well be, I'll see what that function does in that case

[Maxim Levitsky (maximlevitsky)]

Yep, I ported that piece of code to 'C', and this what I get:

User="user" Port="12345/folder" Path="/folder" Host="foo.no-ip.ba

I really feel like rewriting this code properly

#include <stdlib.h>
#include <string.h>
#include <assert.h>

int main()
{
 char uri[] = {"sftp://<email address hidden>:12345/folder" };

 char *u, *h, *s, *p;
 char *h_end;

 assert (uri != NULL);

 u = strchr(uri, ':');
 assert (u != NULL);
 u+=3;

 p = strchr (u, '/');

 h = strchr(u, '@');

 if (h && (p == NULL || h < p)) {
  *h='\0';
  h++;
 } else {
  h = u;
  u = NULL;
 }

 s = strchr(h, ':');

 if (s && (p == NULL || s < p)) {
  h_end = s-1;
  *s = '\0';
  s++;
 } else {
  h_end = p;
  s = NULL;
 }

 if (h_end == NULL) {
  h_end = h + strlen(h);
 }

 printf("User=\"%s\" Port=\"%s\" Path=\"%s\" ",
  u, s, p);

 *h_end = '\0';

 printf("Host=\"%s\n\"", h);
}

[Maxim Levitsky (maximlevitsky)]

Ok, here is the rewrite:

I 'hope' it works

[Tim Besard (maleadt)]

Almost good: the host gets stripped correctly, but doesnt connect to the right port. E.g., opening a sftp browser at "<email address hidden>:12345/data" spawns "ssh <email address hidden> -p 22 -t cd '/folder' && $SHELL -l"
When chaging "port=atoi(tmp)" to "*port=atoi(tmp)", all works perfectly!

[Maxim Levitsky (maximlevitsky)]

How to get that included? It fixes this bug for me

[Iain Lane (laney)]

Is this change upstream? If not, please report a bug and attach the patch there. I'm not comfortable about including without some kind of upstream comment.

[Iain Lane (laney)]

Also if and when it is accepted upstream, please forward the patch to Debian too. Then I will sponsor :)

[Maxim Levitsky (maximlevitsky)]

@Iain Lane

I didn't write this gio port

I have only fixed the function that deals with ssh patchnames.
I just want that this annoying bug will be fixed, and ubuntu made slightly better.

[Iain Lane (laney)]

The best place for such patches to find a home is upstream - that way everyone benefits.

[Iain Lane (laney)]

I just reported it to Debian. After the maintainer responds, we can look at uploading into Ubuntu if he doesn't make a -6 upload straight away. Thanks for your contribution here.

[Christian Neumair (chris-gnome-de)]

I am the Nautilus-Open-Terminal maintainer. It looks like the patch proposed in bug 309529 comment 18 introduces custom URI parsing, instead of reusing the GnomeVFS functions, probably for completely migrating away from GnomeVFS. We should instead wait until the string parsing landed in glib before migrating wway. I understand that the lack of a release for the last few months created an unfortunate situation, sorry for that. I just released Nautilus-Open-Terminal 0.10.

However, regarding the patches: Guys, there is a reason why complex string parsing should be done by libraries instead of hand-crafting it. Judging from the diff [1], the parsing does not look robust. One possibility would be to migrate back to GnomeVFS as in Nautilus-Open-Terminal [2], but I don't know how feasible this is for a stable release.

I am attaching a unit test for the string parsing function used by Ubuntu, to allow further debugging.

[1] http://launchpadlibrarian.net/20583606/nautilus-open-terminal_0.9-3ubuntu1.debdiff
[2] http://git.gnome.org/cgit/nautilus-open-terminal/tree/src/nautilus-open-terminal.c?id=NAUTILUS_OPEN_TERMINAL_0_10#n131

[Christian Neumair (chris-gnome-de)]

The attached file is a simple test application for the routine proposed by Maxim Levitsky. I agree with maleadt's review: It seems to work fine, except for the port parsing which can be fixed via '*port=atoi(tmp);'. Another aspect is that you may want to support the "ssh" scheme, which is equivalent to "sftp". Thanks for your efforts.

Please notice that the patch will not make it upstream.

[Maxim Levitsky (maximlevitsky)]

@Christian Neumair

ssh:// doesn't seem to work at nautilus, only sftp:// does, and old code didn't parse it ether.

I fully agree that path parsing I must say 'must' be done in a library.
Doing so manually like I did, isn't good for maintenance.

But, for a temporary solution I guess that will be ok, like you said until path parsing lands in glib.

Maxim

[Iain Lane (laney)]

Is this still necessary after we got 0.12? I see this:

     + Remote SSH terminal: Work correctly with non-standard SSH ports manually
       defined in ~/.ssh/config

If so, please update the diff (and post it to the Debian bug if you are able) and I will sponsor straight away.

Thanks a lot for your work, it's appreciated.

[Christian Neumair (chris-gnome-de)]

OK, thanks - I just dediced to use Maxim Levitsky's URI parsing implementation upstream, and added a truncation of the password for the case of a sftp://user:pass@host/path URI. That said, the GnomeVFS dependency is now gone. A new 0.13 release was made as well. I think this bug report can be closed.

[Maxim Levitsky (maximlevitsky)]

Bug fixed in karamic, but isn't fixed in jaunty.