Bug 107616 - ripping to fat32 fails with illegal characters in filename
Summary: ripping to fat32 fails with illegal characters in filename
Status: RESOLVED FIXED
Alias: None
Product: k3b
Classification: Unclassified
Component: Audio Project (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Sebastian Trueg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-17 18:01 UTC by Stefan Sieber
Modified: 2006-12-31 00:22 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Dialog with error (44.00 KB, image/png)
2006-11-22 21:37 UTC, Christoph Burger-Scheidlin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Sieber 2005-06-17 18:01:03 UTC
Version:            0.11.23  (using KDE KDE 3.3.2)
Installed from:    Gentoo Packages
Compiler:          gcc 3.3.5.20050130-r1 
OS:                Linux

I got an Error when ripping a track of a CD:
The filename is automatically generated via cddb ("Outkast - Where are my panties?"). I wanted to rip it to mp3 (lame).

When i manually entered the name without '?'(Ripped files Pattern = "%a - Where are my Panties"), the ripping worked well.

Moreover after some testing (entering special characters into the pattern), i found out that the following characters also cause errors (list doesn't need to be complete...):

'|','<', '>', ':'

'*' and '?' cannot be entered in the pattern, which makes sense, because they would also cause errors. But this check can be bypassed when '*' or '?' are contained in the cddb information.

The problem only occurs when ripping to my fat32 partition (Reiserfs3 works well).

Ok, this bug is not very severe. Before my further testing i thought, it would have impact for more people.
Prohibiting these characters for all people would discriminate people with a 'civilized' filesystem.
I think, a more detailed Error - Message would already be enough. The Best solution would be to detect the FS-Type and remove those characters if necessary.
Comment 1 Christoph Burger-Scheidlin 2006-10-05 23:44:32 UTC
Could you please check if this bug still occurs with the recent version of k3b 
(0.12.17 or 1.0pre2)?
Comment 2 Christoph Burger-Scheidlin 2006-11-22 21:36:02 UTC
Confirmed on 1.0beta1. See screenshot.
Comment 3 Christoph Burger-Scheidlin 2006-11-22 21:37:02 UTC
Created attachment 18657 [details]
Dialog with error

Occurs when ripping to smb mounted share, which has the same limitation as a
FAT partition.
Comment 4 Christoph Burger-Scheidlin 2006-11-22 21:38:13 UTC
This is all the debugging output I get:
System
-----------------------
K3b Version: 1.0beta1

KDE Version: 3.5.5
QT Version:  3.3.6
Kernel:      2.6.17-gentoo-r7
Devices
-----------------------
MATSHITA DVD-RAM UJ-841S 1.20 (/dev/sr0, ) [CD-R, CD-RW, CD-ROM, DVD-ROM, DVD-R, DVD-RW, DVD+R, DVD+R DL] [DVD-ROM, DVD-R Sequential, DVD-R Dual Layer Sequential, DVD-RAM, DVD-RW Restricted Overwrite, DVD-RW Sequential, DVD+RW, DVD+R, CD-ROM, CD-R, CD-RW] [SAO, TAO, Restricted Overwrite]

Comment 5 Sebastian Trueg 2006-11-23 10:50:50 UTC
Idea to solve this (basicly just for me to remember it): Integrate filename 
charset checking and correction into K3bFileSplitter and rename it to 
K3bFile.
Comment 6 Sebastian Trueg 2006-12-08 10:55:22 UTC
my solution idea is no good. This way the app does not know which filename is really used which introduces problems for overwriting files especially.
So I have no solution yet and postpone this one to K3b 2.0
Comment 7 Sebastian Trueg 2006-12-31 00:22:35 UTC
SVN commit 618058 by trueg:

Introducing simple FAT filesystem check which replaces invalid characters with "_".

BUG: 107616


 M  +17 -2     libk3b/tools/k3bfilesysteminfo.cpp  
 M  +9 -0      libk3b/tools/k3bfilesysteminfo.h  
 M  +6 -5      src/rip/k3baudiorippingdialog.cpp  
 M  +15 -1     src/rip/videodvd/k3bvideodvdrippingdialog.cpp  
 M  +3 -0      src/rip/videodvd/k3bvideodvdrippingdialog.h  


--- trunk/extragear/multimedia/k3b/libk3b/tools/k3bfilesysteminfo.cpp #618057:618058
@@ -15,8 +15,11 @@
 
 #include "k3bfilesysteminfo.h"
 
+#include <k3bglobals.h>
+
 #include <qfile.h>
 #include <qfileinfo.h>
+#include <qregexp.h>
 
 #include <kdebug.h>
 
@@ -100,8 +103,10 @@
 
 void K3bFileSystemInfo::setPath( const QString& path )
 {
-  d->path = path;
-  d->statDone = false;
+  if( d->path != path ) {
+    d->path = path;
+    d->statDone = false;
+  }
 }
 
 
@@ -111,3 +116,13 @@
     d->stat();
   return d->type;
 }
+
+
+QString K3bFileSystemInfo::fixupPath( const QString& path )
+{
+  QString s = K3b::fixupPath( path );
+  if( type() == K3bFileSystemInfo::FS_FAT )
+    return s.replace( QRegExp("\"?*/\\[]|=:;"), "_" );
+  else
+    return s;
+}
--- trunk/extragear/multimedia/k3b/libk3b/tools/k3bfilesysteminfo.h #618057:618058
@@ -39,6 +39,15 @@
 
   FileSystemType type() const;
 
+  /**
+   * Ensures that the file path does not contain
+   * any invalid chars.
+   *
+   * For now it only replaces characters like * or [
+   * on FAT file systems.
+   */
+  QString fixupPath( const QString& );
+
  private:
   class Private;
   Private* d;
--- trunk/extragear/multimedia/k3b/src/rip/k3baudiorippingdialog.cpp #618057:618058
@@ -25,7 +25,7 @@
 #include <k3bglobals.h>
 #include <k3btrack.h>
 #include <k3bstdguiitems.h>
-
+#include <k3bfilesysteminfo.h>
 #include <k3bpluginmanager.h>
 #include <k3baudioencoder.h>
 
@@ -74,6 +74,7 @@
 
   QValueVector<QString> filenames;
   QString playlistFilename;
+  K3bFileSystemInfo fsInfo;
 };
 
 
@@ -289,6 +290,7 @@
   d->filenames.clear();
 
   QString baseDir = K3b::prepareDir( m_optionWidget->baseDir() );
+  d->fsInfo.setPath( baseDir );
 
   KIO::filesize_t overallSize = 0;
 
@@ -333,9 +335,8 @@
 			     K3b::Msf(length).toString(),
 			     fileSize < 0 ? i18n("unknown") : KIO::convertSize( fileSize ),
 			     i18n("Audio") );
+    d->filenames.append( d->fsInfo.fixupPath( baseDir + "/" + filename + "." + extension ) );
 
-    d->filenames.append( K3b::fixupPath( baseDir + "/" + filename + "." + extension ) );
-
     if( m_optionWidget->createCueFile() )
       (void)new KListViewItem( m_viewTracks,
 			       m_viewTracks->lastItem(),
@@ -391,7 +392,7 @@
 			       fileSize < 0 ? i18n("unknown") : KIO::convertSize( fileSize ),
 			       (m_toc[index].type() == K3bTrack::AUDIO ? i18n("Audio") : i18n("Data") ) );
 
-      d->filenames.append( K3b::fixupPath( baseDir + "/" + filename ) );
+      d->filenames.append( d->fsInfo.fixupPath( baseDir + "/" + filename ) );
     }
   }
 
@@ -409,7 +410,7 @@
 			     "-",
 			     i18n("Playlist") );
     
-    d->playlistFilename = K3b::fixupPath( baseDir + "/" + filename );
+    d->playlistFilename = d->fsInfo.fixupPath( baseDir + "/" + filename );
   }
 
   if( overallSize > 0 )
--- trunk/extragear/multimedia/k3b/src/rip/videodvd/k3bvideodvdrippingdialog.cpp #618057:618058
@@ -21,6 +21,7 @@
 #include <k3bmedium.h>
 #include <k3bmediacache.h>
 #include <k3bglobals.h>
+#include <k3bfilesysteminfo.h>
 
 #include <klocale.h>
 #include <klistview.h>
@@ -136,6 +137,13 @@
 };
 
 
+class K3bVideoDVDRippingDialog::Private
+{
+public:
+  K3bFileSystemInfo fsInfo;
+};
+
+
 K3bVideoDVDRippingDialog::K3bVideoDVDRippingDialog( const K3bVideoDVD::VideoDVD& dvd, 
 						    const QValueList<int>& titles,
 						    QWidget* parent, const char* name )
@@ -147,6 +155,8 @@
 			  "VideoDVD Ripping" ), // config group 
     m_dvd( dvd )
 {
+  d = new Private;
+
   QWidget* frame = mainWidget();
   QHBoxLayout* frameLayout = new QHBoxLayout( frame );
   frameLayout->setMargin( 0 );
@@ -171,6 +181,7 @@
 
 K3bVideoDVDRippingDialog::~K3bVideoDVDRippingDialog()
 {
+  delete d;
 }
 
 
@@ -244,12 +255,15 @@
 
 void K3bVideoDVDRippingDialog::slotUpdateFilenames()
 {
+  QString baseDir = K3b::prepareDir( m_w->m_editBaseDir->url() );
+  d->fsInfo.setPath( baseDir );
+
   for( QMap<QCheckListItem*, K3bVideoDVDRippingJob::TitleRipInfo>::iterator it = m_titleRipInfos.begin();
        it != m_titleRipInfos.end(); ++it ) {
     QString f = createFilename( it.data(), m_w->m_comboFilenamePattern->currentText() );
     if( m_w->m_checkBlankReplace->isChecked() )
       f.replace( QRegExp( "\\s" ), m_w->m_editBlankReplace->text() );
-    it.data().filename = K3b::prepareDir( m_w->m_editBaseDir->url() ) + f;
+    it.data().filename = d->fsInfo.fixupPath( baseDir + f );
     it.key()->setText( 3, f );
   }
 }
--- trunk/extragear/multimedia/k3b/src/rip/videodvd/k3bvideodvdrippingdialog.h #618057:618058
@@ -74,6 +74,9 @@
   QMap<QCheckListItem*, K3bVideoDVDRippingJob::TitleRipInfo> m_titleRipInfos;
 
   class AudioStreamViewItem;
+
+  class Private;
+  Private* d;
 };
 
 #endif