Incorrect storage of authenticated stream URLS

Bug #133340 reported by m4dm4n
6
Affects Status Importance Assigned to Milestone
Amarok
Invalid
Undecided
Unassigned
amarok (Ubuntu)
Invalid
Low
Unassigned

Bug Description

Binary package hint: amarok

When adding streams which employ HTTP authentication via a username/password included in the URL in the format http://username:<email address hidden>, Amarok will successfully play them until either the program is exited or another playlist is loaded. After this happens, if the stream is played again, it will present an error saying no suitable decoder could be found. If the stream entry is then edited it shows that there is no longer a password present, only a username. If the password is re-added and the entry is saved and played it works again. I believe the error is due to an authentication failure since the password is missing.

I have tracked down the issue which I believe to be caused by a call to KURL::prettyURL() in StreamEntry::xml() through: url.appendChild( doc.createTextNode( m_url.prettyURL() )) in playlistbrowseritem.cpp on line 1197 (1.4.6)
Note that in the kdecore documentation it states that KURL::prettyURL will remove the password entry from the URL, since it is being formatted for display purposes. As a result the stream entry is saved to the stream database without a password present in the URL.

The issue presented in 1.4.6, although it may have existed before then. This functionality may or may not be intentional, however in the case that it is intentional I don't believe it is clear enough to the user that they will need to re-add the password to resolve the problem.

Revision history for this message
Jeff Mitchell (jefferai) wrote :

I'd like to fix this. Are you building from source? If so, can you see if it all works well by using .url() instead of .prettyURL()?

I don't have any access to any music location using username:password; alternately if you give me access to something like that I can test it out myself.

Thanks for tracking this down!

Revision history for this message
Jeff Mitchell (jefferai) wrote :

Note (more to myself than you) that this prettyURL issue pops up in a few places in that file for a few different types of playlist items, and needs to be changed in all those places.

m4dm4n: you may want to watch debug output and the like, make sure that the password isn't showing up in there anywhere.

Revision history for this message
Jeff Mitchell (jefferai) wrote :

Any progress on this?

Revision history for this message
Lydia Pintscher (lydia-pintscher) wrote :

Marking as incomplete.
Please give feedback.

Changed in amarok:
status: New → Incomplete
Revision history for this message
m4dm4n (m4dm4n) wrote :

Okay this bug has suffered neglect on my part - sorry about that. : (

I've just rebuilt from source with the change applied to the latest 1.4.8 release and I can confirm that this does fix the problem I described and builds successfully. The user's password may risk being exposed in several places, but thats really the nature of these types of URLs and I think that the risk of exposing the password is less important than the inconvenience of an - from the users point of view - unrelated error message about not being able to find a suitable decoder. I suspect that this is a fairly rare requirement anyway, but it works for me at least.

To fix the bug, I have changed the KURL::prettyURL() call to a regular KURL::url() call on line 1201 in the new release. I think that this is more appropriate here regardless of the bug, since prettyURL is for displaying the URL and in most cases any changes would probably make it unusable in practice, anyway.

Sorry about dropping the ball on this one.

Revision history for this message
m4dm4n (m4dm4n) wrote :

Sorry, sent that last message a bit prematurely. It needs a bit of a closer investigation, which I'll get back to you on. I think that this may be cropping up in a few other places, not just in saving the URLs.

Changed in amarok:
importance: Undecided → Low
status: Incomplete → Triaged
Revision history for this message
Jonathan Thomas (echidnaman) wrote :

Upstream is not accepting bugs for Amarok 1.4.x due tot he focus on Amarok2, so invalidating upstream task.

Changed in amarok:
status: New → Invalid
Revision history for this message
Jonathan Thomas (echidnaman) wrote :

Is this still a problem with Amarok 2.0.1.1?

Changed in amarok:
status: Triaged → Incomplete
Revision history for this message
Jonathan Thomas (echidnaman) wrote :

We are closing this bug report because it lacks the information we need to investigate the problem, as described in the previous comments. Please reopen it if you can give us the missing information, and don't hesitate to submit bug reports in the future. To reopen the bug report you can click on the current status, under the Status column, and change the Status back to "New". Thanks again!

Changed in amarok:
status: Incomplete → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.