don't set a user if there is no user
ClosedPublic

Authored by sitter on Oct 17 2018, 3:18 PM.

Details

Summary

it doesn't have technical downsides but ends up creating urls of the type

scheme://@host/path

which is technically equal to scheme://host/path, so the excess @ is really
just not necessary. simply check if the username is empty and if so
do not set a username on the QUrl

Test Plan
  • no useless @
  • setting up fish connection with and without username still works (without defaults to local username)

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3987
Build 4005: arc lint + arc unit
sitter created this revision.Oct 17 2018, 3:18 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 17 2018, 3:18 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Oct 17 2018, 3:18 PM
broulik accepted this revision.Oct 17 2018, 3:20 PM

I wonder why QUrl doesn't clean that up on its own

knetattach/knetattach.cpp
204

lazy, const QString

This revision is now accepted and ready to land.Oct 17 2018, 3:20 PM

I think ... since this gets put into the desktop file via toDisplayString and toDisplayString has the behavior that

With the default options, the resulting QString can be passed back to a QUrl later on, but any password that was present initially will be lost.

what QUrl does is probably accurate. In that an empty QString is not a null QString, so if the goal is to reconstruct the same QUrl out of the string then encoding the empty username like that is reasonable. I've had a quick grep through rfc2396 and from a spec POV @host == host though.

This revision was automatically updated to reflect the committed changes.