Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate segfaults if path contains a non-existant node #587

Closed
ASLeonard opened this issue Aug 9, 2024 · 2 comments
Closed

validate segfaults if path contains a non-existant node #587

ASLeonard opened this issue Aug 9, 2024 · 2 comments

Comments

@ASLeonard
Copy link

This is likely an obscure corner case that is hard to protect against, but may be useful. I'm not sure if it is the responsibility of odgi to gracefully tell people their graphs are malformed rather than crashing, but there is at least a partial issue here.

Running odgi validate on a gfa where the path includes a non-existant node causes a segfault.

S       1       A
S       2       C
S       3       T
S       4       G
L       1       +       2       +       0M
L       1       +       3       +       0M
L       2       +       4       +       0M
P       foo     1+,2+,4+        0M
P       bar     1+,5+   0M

Arguably, the description of the subcommand should tell the user the paths are not consistent with the graph (since no path can contain node 5 since node 5 doesn't exist). This currently only checks for consistency with L-lines assuming L-lines are already consistent with S-lines.

Interestingly, a gfa with a link line containing a non-existant node raises a "Floating point exception"

S       1       A
S       2       C
S       3       T
S       4       G
L       1       +       2       +       0M
L       1       +       3       +       0M
L       2       +       4       +       0M
L       2       +       5       +       0M
P       foo     1+,2+,4+        0M
P       bar     1+,3+   0M

This can be guarded against with some checks in src/gfa_to_handle.cpp, namely for edges here

handlegraph::handle_t a = graph->get_handle(stol(e.source_name) - id_increment, !e.source_orientation_forward);
handlegraph::handle_t b = graph->get_handle(stol(e.sink_name) - id_increment, !e.sink_orientation_forward);
graph->create_edge(a, b);

and presumably for paths here

odgi/src/gfa_to_handle.cpp

Lines 127 to 129 in 2151dd8

graph->get_handle(id,
// in gfak, true == +
!p->gfak.orientations[i++]));

Unsurprisingly since the issue originates in the gfa loading, almost all subcommands segfault if the path contains a non-existant node.

Best,
Alex

@AndreaGuarracino
Copy link
Member

@ASLeonard, thank you for breaking ODGI xD I've put a few polite checks in #591. Could you try to break it again?

@ASLeonard
Copy link
Author

The changes handle all the bad files I had. And they print a very useful message for the error as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants