Bug 92363 - Hexadecimal character conversion in attachement URIs not handled properly
Summary: Hexadecimal character conversion in attachement URIs not handled properly
Status: RESOLVED FIXED
Alias: None
Product: korganizer
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Reinhold Kainhofer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-10-30 01:25 UTC by Anton Markov
Modified: 2007-03-01 04:51 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Simple fix to reported problem (807 bytes, patch)
2004-12-24 02:24 UTC, Anton Markov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Markov 2004-10-30 01:25:34 UTC
Version:            (using KDE KDE 3.3.0)
Installed from:    Debian testing/unstable Packages
OS:                Linux

When creating an attachment to an event/task, special characters in the URI are converted to hexadecimal form (%xx). When the URI is editted, the hex are not converted back to alpha-numeric, and the % sign is also converted to hex. See the "Steps to reproduce" below for a better expanation.

Steps to reproduce:
1) Click on the "Attachments" tab in the "Edit Event" or "Edit To-Do" window.
2) Click "Add..." and enter a URI with a space or other non-standard character. For example, "file://path/to/my file" (does not have to be a real file).
3) Press "OK"

* Result: The program replaces the space with the hex code "%20". So the URI above becomes, "file://path/to/my%20file". This is messy and confusing but correct. Perhaps the URI should be converted to human-readable form before display.

4) Select the new attachment and press "Edit"
* The spaces in the edit box are replaced with "%20".
5) Select "OK" again.
* Expected: The "%20" should be converted to a space or remain "%20"
* Result: The "%" sign is converted to hex (%25). URI becomes corrupt: "file://path/to/my%2520file". Repeating steps 4-5 makes the problem worse by converting the % sign again.

Proposed solution:
Store URI with hex characters internally, but convert to alpha-numeric when displaying in the list of attachments or editting.
Comment 1 Anton Markov 2004-12-24 02:24:19 UTC
Created attachment 8799 [details]
Simple fix to reported problem

This is a simple one-line patch which fixes this bug. 

It changes KOEditorAttachments::slotEdit() to pass the stored URL through
KURL::decode_string() before passing it to KURLReqKURLRequesterDlg::getURL().

This bug was a result of KURLRequesterDlg::getURL() not decoding the URL passed
to it by the program, but encoding the URL when returning. Thus it ends up
encoding the URL multiple times if you edit the URL more than once. This patch
adds the missing decoding step.
Comment 2 David Faure 2004-12-24 10:38:56 UTC
On Friday 24 December 2004 02:44, Anton Markov wrote:
> It changes KOEditorAttachments::slotEdit() to pass the stored URL 
> through KURL::decode_string() before passing it to 
> KURLReqKURLRequesterDlg::getURL().

This is the wrong approach. It is never necessary to call encode_string or decode_string.

Usually there is a path vs url confusion - it's often that the code does
KURL u( somepath ) instead of KURL u; u.setPath( somepath ).
But in this code I see only URLs, no paths, so it must be something else.
This is about URLs all the way, right?
Can you print out for me the exact values of the url before and after the
getURL call?

One small improvement could be  item->setText( 0, uri.prettyURL() );
instead of  item->setText( 0, uri.url() );  but this probably isn't enough
to fix a double-escaping bug.

Comment 3 Anton Markov 2004-12-24 23:34:34 UTC
As far as I understand, KOrganizer stores a URL internally. It makes sense for the URL to be stored in encoded form. The root of the problem is that KURLRequesterDlg::getURL() does not decode the URL KOrganizer passes to it before displaying it in the "Location" edit box (opening the file selector decodes the URL properly). When the KURLRequesterDlg::getURL() dialogue is closed, it encodes the URL, including any remaining "%" signs which remain from the first time the values where escaped.

This is most appearant with white-spaces, but applies to all characters not allowed in the  URL. Example:

I enter the following into the URL box: "/home/anton/some file.txt"
After pressing 'OK': "/home/anton/some%20file.txt"
After pressing 'Edit' and 'OK' again: "/home/anton/some%2520file.txt"

It looks like the KURLRequesterDlg::getURL() function expects a decoded URL, but outputs an encoded one.

Or perhaps it _is_ a path vs. URL confusion. The documentation for KURLRequesterDlg::getURL() says this about the "url" argument:

url  This specifies the initial path of the input line. 

If it really does expect a path, then we should use the following to pass the URL to KURLRequesterDlg::getURL():

KURL::fromPathOrURL( item->text( 0 ) )->path()

This is probably the best thing to do.

As for using prettyURL(), that would result in some of the URL informating (passwords, certain escapes) being lost. Since the QListView is the only copy of the data, this is a bad idea. The best solution would be to store a separate master copy of the URLs in a QPtrList<QString> or a custom class, and only use the QListView to display the information through prettyURL().
Comment 4 Anton Markov 2004-12-25 00:55:21 UTC
I just tried using "KURL::fromPathOrURL( item->text( 0 ) ).path()" and it only returns the path. i.e. it works for local files but for other URLs it returns only the directory on the server. For example:

If I enter: "http://www.google.com/linux"
and then go edit the URL again I get "/linux".

I don't see another way except for decode_string(). That is unless we want to modify KURLRequesterDlg::getURL() to decode the URL internally.
Comment 5 Reinhold Kainhofer 2005-06-26 15:09:25 UTC
This works now. When you press "Edit..." for the attachment, the editor dlg uses a space instead of the %20.

Cheers,
Reinhold
Comment 6 Olivier Vitrat 2007-03-01 04:51:05 UTC
I still have the problem in version 3.5.6