View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0003991Slicer4Core: GUIpublic2015-04-26 08:272017-06-10 08:51
Reporterpinter Assigned Tonicole  
PrioritynormalSeveritycrashReproducibilitysometimes
Status closedResolutionfixed 
Product VersionSlicer 4.4.0 
Target VersionFixed in VersionSlicer 4.6.0 
Summary0003991: Node names in form "1: Something" confuse save dialog, lead to crash
Description

Happens on Windows

  1. Load MRHead
  2. Rename it to "1: MRHead"
  3. Click Save button

It does not always crash (it does with one of my test data), but it is reproducible on Windows.

20150426_SaveDialogIssue.png shows the non-crash scenario,
20150426_SaveFileCrash.png shows where it crashes
The last Slicer call is line 534 in qSlicerSaveDataDialog.cxx:
snode->SetFileName(fileName.absoluteFilePath().toLatin1());

Additional Information

Calling function nativeAbsoluteFilePath in QFileSystemEntry QFileSystemEngine::absoluteName(const QFileSystemEntry &entry) makes wrong substitutions in file name, for example it changes "3: RTSTRUCT.seg" to "3:\ RTSTRUCT.seg", which leads to crash in qfilesystemengine_win.cpp, line 565 (Q_ASSERT(ret.at(0).isLetter());)

TagsNo tags attached.

Activities

2015-04-26 08:27

 

20150426_SaveDialogIssue.png (375,985 bytes)

2015-04-26 08:28

 

20150426_SaveFileCrash.png (34,555 bytes)
pinter

pinter

2015-04-26 08:56

developer   ~0013024

Only crashes in debug mode, on release it just confuses the dialog as shown in one of the screenshots.
pieper

pieper

2015-10-23 12:42

administrator   ~0013406

Colon is not a file system safe character (it's a volume specifier on windows). Probably we should escape that character and any similar ones. It would make sense for Qt to provide a high level interface for this, but I've never found one.

In the past I've used this heuristic:

https://github.com/pieper/dicomsort/blob/master/dicomsort.py#L102

Is there something better that can be done?
pinter

pinter

2015-10-24 09:35

developer   ~0013422

Thanks Steve! Escaping the file name makes sense, and I cannot think of anything better right now. We should probably go ahead and implement it in file save dialog.
pieper

pieper

2015-11-01 08:47

administrator   ~0013472

Hi Csaba -

I could not replicate this with the current build on windows - is it still an issue for you?

-Steve
pinter

pinter

2015-11-01 15:38

developer   ~0013473

Thanks for taking the time to look at this issue!
Your test run reminded me that I had applied a workaround back in revision
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24294
so that at least there is no crash, see around line
https://github.com/Slicer/Slicer/blob/master/Base/QTGUI/qSlicerSaveDataDialog.cxx#L493
Sorry for not referencing the Mantis issue! If this crude but benign solution is fine, then we can close this ticket.
Unfortunately save data dialog has a number of issues due to its complexity (I discovered some when working on segmentations storage, like the re-use of the fileinfo object, see https://www.assembla.com/spaces/slicerrt/tickets/758), so I think it will eventually need an overhaul.
pieper

pieper

2015-11-03 07:57

administrator   ~0013526

Agreed - there's no doubt a better overall solution but as long as this works to avoid the crash it's a good step.
pinter

pinter

2015-11-03 10:25

developer   ~0013532

Steve, Jc @ hangout: The best fix would be to "sanitize" file names before saving by replacing a list of "bad" characters with underscores
nicole

nicole

2015-11-10 13:23

administrator   ~0013582

This fix causes a crash when saving nodes with names like "1" or "4", pull request to avoid the crash:
https://github.com/Slicer/Slicer/pull/404
pinter

pinter

2015-11-10 13:41

developer   ~0013583

Makes perfect sense, thanks!
nicole

nicole

2015-11-11 06:54

administrator   ~0013588

Fix is in svn:
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24722
pinter

pinter

2016-02-22 15:32

developer   ~0013821

The reason I haven't closed this issue yet is the suggestion in comment
http://www.na-mic.org/Bug/view.php?id=3991#c13532
Should we aim to do that or should I just close the issue?
pieper

pieper

2016-02-23 09:49

administrator   ~0013822

Hi Csaba - maybe close this and start a child issue as a feature request to sanitize file names? We might want to urlencode illegal characters. -Steve

Related Changesets

Import 2017-06-07 23:51:09: master 088b8182

2015-11-11 11:15:34

naucoin

Details Diff 		BUG: Fix crash on saving node named with a single number

A previous fix[1] to avoid crashes on windows when saving
volumes named "1:xxxx" leads to a crash when a node name
is just a single number (the first part of the check passes, but
crashes when trying to get the second character in the name string).
This fix adds a string length check so that valid node names like
"1" or "2" will not cause a crash on save.

[1] http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24294

Issue 0003991

git-svn-id: http://svn.slicer.org/Slicer4/trunk@24722 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Base/QTGUI/qSlicerSaveDataDialog.cxx Diff File

Issue History

Date Modified Username Field Change
2015-04-26 08:27 pinter New Issue
2015-04-26 08:27 pinter File Added: 20150426_SaveDialogIssue.png
2015-04-26 08:28 pinter File Added: 20150426_SaveFileCrash.png
2015-04-26 08:56 pinter Note Added: 0013024
2015-10-23 12:42 pieper Note Added: 0013406
2015-10-24 09:35 pinter Note Added: 0013422
2015-11-01 08:47 pieper Note Added: 0013472
2015-11-01 15:38 pinter Note Added: 0013473
2015-11-03 07:57 pieper Note Added: 0013526
2015-11-03 10:25 pinter Note Added: 0013532
2015-11-10 13:23 nicole Note Added: 0013582
2015-11-10 13:41 pinter Note Added: 0013583
2015-11-11 06:54 nicole Note Added: 0013588
2016-02-22 15:22 nicole Status new => resolved
2016-02-22 15:22 nicole Resolution open => fixed
2016-02-22 15:22 nicole Assigned To => nicole
2016-02-22 15:32 pinter Note Added: 0013821
2016-02-23 09:49 pieper Note Added: 0013822
2016-02-23 10:41 pinter Status resolved => closed
2016-02-23 10:41 pinter Fixed in Version => Slicer 4.5.1
2016-10-13 02:00 jcfr Fixed in Version Slicer 4.5.1 => Slicer 4.6.0
2017-06-10 08:51 Changeset attached => Slicer master 088b8182