Bug 130849 - fails to handle long URLs nicely due to Filename size limitations
Summary: fails to handle long URLs nicely due to Filename size limitations
Status: RESOLVED FIXED
Alias: None
Product: akregator
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-15 05:03 UTC by Daniel Black
Modified: 2006-08-21 00:40 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Black 2006-07-15 05:03:03 UTC
Version:           1.2.3 (using KDE KDE 3.5.3)
Installed from:    Gentoo Packages
Compiler:          3.4.6 
OS:                Linux

When attaching large URL to akgregator, the filesystem is unable to handle the URL. The below URL is taken from a bugs.gentoo.org query and then selecting the RSS feed option on the bottom.

https://bugs.gentoo.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=allwordssubstr&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=security%40gentoo.org&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=dragonheart%40gentoo.org&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&field0-0-0=noop&type0-0-0=noop&value0-0-0=&ctype=rss

A strace through the program showed:

[pid 25272] open("/var/tmp/kdecache-dan/akregator/Media/https___bugs.gentoo.org_buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=allwordssubstr&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=security%40gentoo.org&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=dragonheart%40gentoo.org&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&field0-0-0=noop&type0-0-0=noop&value0-0-0=&ctype=rss.png", O_RDONLY|O_LARGEFILE) = -1 ENAMETOOLONG (File name too long)

While this is still working currently akregator was freezing on startup. After removing long URLS from /home/dan/.kde3.5/share/apps/akregator/data//feeds.opml akregator was functioning fine.

If this freezing occurs again I'll include a strace/backtrace and upgrade severity to crash.

thought: handle long urls as a hash of the URL rather than the name itself.
Comment 1 Frank Osterfeld 2006-08-20 20:32:26 UTC
> thought: handle long urls as a hash of the URL rather than the name itself.

We already do that in other places, first 200 chars of url + hash(url).
Need to move that into an own method and use it thorough the code.
Comment 2 Frank Osterfeld 2006-08-21 00:27:30 UTC
SVN commit 575176 by osterfeld:

when using escaped URLs as filename, make sure the filename doesn't get longer than 255 chars.
moved the "escaping" code to Utils::fileNameForUrl().
BUG: 130849


 M  +2 -4      articleviewer.cpp  
 M  +7 -4      feed.cpp  
 M  +12 -0     utils.cpp  
 M  +8 -0      utils.h  


--- branches/KDE/3.5/kdepim/akregator/src/articleviewer.cpp #575175:575176
@@ -428,8 +428,7 @@
 
     if (feed && !feed->image().isNull())
     {
-        QString url=feed->xmlUrl();
-        QString file = url.replace("/", "_").replace(":", "_");
+        QString file = Utils::fileNameForUrl(feed->xmlUrl());
         KURL u(m_imageDir);
         u.setFileName(file);
         text += QString("<a href=\"%1\"><img class=\"headimage\" src=\"%2.png\"></a>\n").arg(feed->htmlUrl()).arg(u.url());
@@ -524,8 +523,7 @@
 
     if (feed && !feed->image().isNull())
     {
-        QString url=feed->xmlUrl();
-        QString file = url.replace("/", "_").replace(":", "_");
+        QString file = Utils::fileNameForUrl(feed->xmlUrl());
         KURL u(m_imageDir);
         u.setFileName(file);
         text += QString("<a href=\"%1\"><img class=\"headimage\" src=\"%2.png\"></a>\n").arg(feed->htmlUrl()).arg(u.url());
--- branches/KDE/3.5/kdepim/akregator/src/feed.cpp #575175:575176
@@ -46,6 +46,7 @@
 #include "feedstorage.h"
 #include "storage.h"
 #include "treenodevisitor.h"
+#include "utils.h"
 
 #include "librss/librss.h"
 
@@ -205,8 +206,9 @@
 
 void Feed::loadImage()
 {
-    QString u = d->xmlUrl;
-    QString imageFileName = KGlobal::dirs()->saveLocation("cache", "akregator/Media/") + u.replace("/", "_").replace(":", "_")+".png";
+    QString imageFileName = KGlobal::dirs()->saveLocation("cache", "akregator/Media/") 
+                            + Utils::fileNameForUrl(d->xmlUrl) + 
+".png";
     d->imagePixmap.load(imageFileName, "PNG");
 }
         
@@ -585,8 +587,9 @@
     if (image.isNull())
         return;
     d->imagePixmap=image;
-    QString u = d->xmlUrl;
-    d->imagePixmap.save(KGlobal::dirs()->saveLocation("cache", "akregator/Media/")+u.replace("/", "_").replace(":", "_")+".png","PNG");
+    d->imagePixmap.save(KGlobal::dirs()->saveLocation("cache", "akregator/Media/") 
+                        + Utils::fileNameForUrl(d->xmlUrl) + 
+".png","PNG");
     nodeModified();
 }
 
--- branches/KDE/3.5/kdepim/akregator/src/utils.cpp #575175:575176
@@ -44,4 +44,16 @@
     return hash;
 }
 
+QString Utils::fileNameForUrl(const QString& url_p)
+{
+    QString url2(url_p);
+    
+    url2 = url2.replace("/", "_").replace(":", "_");
+    
+    if (url2.length() > 255)
+        url2 = url2.left(200) + QString::number(Akregator::Utils::calcHash(url2), 16);
+    
+    return url2;
 }
+
+}
--- branches/KDE/3.5/kdepim/akregator/src/utils.h #575175:575176
@@ -43,6 +43,14 @@
     */
 
     static uint calcHash(const QString& str);
+    
+    /**
+     * returns a file name for a URL, with chars like "/" ":"
+     * replaced by "_". Too long URLs (>255 chars) are shortened and
+     * appended with a hash value.
+     * 
+     */
+    static QString fileNameForUrl(const QString& url);
 };
 
 }
Comment 3 Frank Osterfeld 2006-08-21 00:40:45 UTC
SVN commit 575183 by osterfeld:

forwardport #130849 (handle large URLs when using them as filename)
CCBUG: 130849


 M  +3 -6      articleviewer.cpp  
 M  +4 -3      feed.cpp  
 M  +12 -0     utils.cpp  
 M  +8 -0      utils.h  


--- trunk/KDE/kdepim/akregator/src/articleviewer.cpp #575182:575183
@@ -80,8 +80,7 @@
         if (!node->image().isNull()) // image
         {
             text += QString("<div class=\"body\">");
-            QString url=node->xmlUrl();
-            QString file = url.replace("/", "_").replace(":", "_");
+            QString file = Utils::fileNameForUrl(node->xmlUrl());
             KUrl u(KUrl::fromPath(m_view->m_imageDir.toString()));
             u.setFileName(file);
             text += QString("<a href=\"%1\"><img class=\"headimage\" src=\"%2.png\"></a>\n").arg(node->htmlUrl()).arg(u.url());
@@ -426,8 +425,7 @@
 
     if (feed && !feed->image().isNull())
     {
-        QString url=feed->xmlUrl();
-        QString file = url.replace("/", "_").replace(":", "_");
+        QString file = Utils::fileNameForUrl(feed->xmlUrl());
         KUrl u(KUrl::fromPath(m_imageDir.toString()));
         u.setFileName(file);
         text += QString("<a href=\"%1\"><img class=\"headimage\" src=\"%2.png\"></a>\n").arg(feed->htmlUrl()).arg(u.url());
@@ -523,8 +521,7 @@
 
     if (feed && !feed->image().isNull())
     {
-        QString url=feed->xmlUrl();
-        QString file = url.replace("/", "_").replace(":", "_");
+        QString file = Utils::fileNameForUrl(feed->xmlUrl());
         KUrl u(KUrl::fromPath(m_imageDir.toString()));
         u.setFileName(file);
         text += QString("<a href=\"%1\"><img class=\"headimage\" src=\"%2.png\"></a>\n").arg(feed->htmlUrl()).arg(u.url());
--- trunk/KDE/kdepim/akregator/src/feed.cpp #575182:575183
@@ -33,6 +33,7 @@
 #include "folder.h"
 #include "storage.h"
 #include "treenodevisitor.h"
+#include "utils.h"
 #include <syndication/syndication.h>
 
 #include <kdebug.h>
@@ -609,7 +610,8 @@
     if (d->imagePixmap.isNull())
     {
         QString u = d->xmlUrl;
-        QString imageFileName = KGlobal::dirs()->saveLocation("cache", "akregator/Media/")+u.replace("/", "_").replace(":", "_")+".png";
+        QString imageFileName = KGlobal::dirs()->saveLocation("cache", "akregator/Media/")
+                                + Utils::fileNameForUrl(d->xmlUrl) + ".png";
         d->imagePixmap=QPixmap(imageFileName, "PNG");
 
         // if we aint got teh image and the feed provides one, get it....
@@ -687,8 +689,7 @@
     if (p.isNull())
         return;
     d->imagePixmap=p;
-    QString u = d->xmlUrl;
-    d->imagePixmap.save(KGlobal::dirs()->saveLocation("cache", "akregator/Media/")+u.replace("/", "_").replace(":", "_")+".png","PNG");
+    d->imagePixmap.save(KGlobal::dirs()->saveLocation("cache", "akregator/Media/")+ Utils::fileNameForUrl(d->xmlUrl) + ".png","PNG");
     nodeModified();
 }
 
--- trunk/KDE/kdepim/akregator/src/utils.cpp #575182:575183
@@ -44,4 +44,16 @@
     return hash;
 }
 
+QString Utils::fileNameForUrl(const QString& url_p)
+{
+    QString url2(url_p);
+    
+    url2 = url2.replace("/", "_").replace(":", "_");
+    
+    if (url2.length() > 255)
+        url2 = url2.left(200) + QString::number(Akregator::Utils::calcHash(url2), 16);
+    
+    return url2;
 }
+
+}
--- trunk/KDE/kdepim/akregator/src/utils.h #575182:575183
@@ -45,6 +45,14 @@
     */
 
     static uint calcHash(const QString& str);
+    
+    /**
+     * returns a file name for a URL, with chars like "/" ":"
+     * replaced by "_". Too long URLs (>255 chars) are shortened and
+     * appended with a hash value.
+     * 
+     */
+    static QString fileNameForUrl(const QString& url);
 };
 
 } // namespace Akregator