Force KAuth helpers to have UTF-8 support
ClosedPublic

Authored by davidedmundson on Sep 8 2017, 12:09 PM.

Details

Reviewers
apol
aacid
Summary

Our helper runs as root and doesn't have the user's environment
(including $LANG). This defaults to one without UTF-8.

Without a locale with UTF-8
QFileInfo::exists("/backgrounds/crepé.png") returns false.

This causes problems in the SDDM KCM's code to set a wallpaper. (and
probably elsewhere)

By forcing the text codec we can ignore the locale

BUG: 384294

Test Plan

Tested case in bug report setting a wallpaper from the SDDM KCM

Diff Detail

Repository
R283 KAuth
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10102
Build 10120: arc lint + arc unit
davidedmundson created this revision.Sep 8 2017, 12:09 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 8 2017, 12:09 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript

C.UTF-8 is not glibc, even few distributions added it (historically Debian and Ubuntu, more recently Fedora).

:(

Though worst case it'll fallback to C which is no worse than where we started.

@ltoscano Do you know of any other solution?

aacid added a subscriber: aacid.Sep 9 2017, 9:59 AM

:(

Though worst case it'll fallback to C which is no worse than where we started.

@ltoscano Do you know of any other solution?

Pass the locale from the kauth caller? or is it too late to set up LANG at that point?

It's too late*, if we're processing stuff from the user we've already instantiated our qapp and set our locale/test codec.

  • It would be possible to do this and then have this helper spawn another helper ... but that gets messy
aacid added a comment.Sep 9 2017, 11:20 AM

Then invoke locale -a and get one of the utf locales from there? It sucks but i didn't find a way to do what locale -a does from C[++]

Rewrote using QTextCodec

Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptMar 1 2019, 6:19 PM
davidedmundson edited the summary of this revision. (Show Details)Mar 1 2019, 6:20 PM

pedantic change

aacid added a comment.Mar 1 2019, 6:40 PM

I'm not sure changing the codec for all kauth stuff is the best, but if we're going to do that i think we should add it to the documentation, something like "Forces UTF-8 text codec" or something

I'm not sure changing the codec for all kauth stuff is the best

I did it in just my one helper initially, but in review it was suggested to make it global as it affects everyone. I'm content with either as long as I'm not deadlocked between them.

aacid added a comment.Mar 5 2019, 5:57 PM

I'm not sure changing the codec for all kauth stuff is the best

I did it in just my one helper initially, but in review it was suggested to make it global as it affects everyone. I'm content with either as long as I'm not deadlocked between them.

Ok, then get whoever told you yo do it for all to review this ;)

src/kauthactionreply.h
195

it should either say "unix only" here or have move the code outside the ifdef, i really don't know what's best, guess it's better if we play it safe and make it unix only?

apol added a comment.Mar 25 2019, 2:08 PM

LGTM +1
I've dealt with such issues when working on samba-mounter, there we ended up encoding everything as base64 and using QString::fromUtf8 like maniacs.

aacid accepted this revision.Mar 25 2019, 7:29 PM
This revision is now accepted and ready to land.Mar 25 2019, 7:29 PM

This was merged, phabricator ignored it for some reason