[MacPorts] #49721: dangerous bug in Qt5
#49721: dangerous bug in Qt5 -------------------------+-------------------------------- Reporter: rjvbertin@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: Keywords: | Port: qt5 -------------------------+-------------------------------- Last week I ran into a dangerous bug in Qt5, as a result of which I had to restore a large part of /Applications from backup. The bug is in QtCore::QStandardPaths (QSP) which provides applications with a way to obtain the paths to a range of standard locations, in readable ("system") and writable ("user") form. The class also has a switch to put it into testing mode that changes the returned locations, intended for use in unittests/autotests so that they don't overwrite or delete anything in the actual locations. On OS X, QSP returns locations that are compliant with the OS X way of doing things (app. data into ~/Library/Application Support, for instance) and App Store rules. For the `ApplicationsLocation` it returns `/Applications`, which seems reasonable (but in fact has nothing to do with the role of the ApplicationsLocation on Linux). This location makes no difference between readable and writable, and more importantly, not either between regular and testing mode. As a result, software tests that verify functionality involving reading and writing from ApplicationsLocation will - attempt to get the testing ApplicationsLocation - store files there, do stuff with them - cleanup afterwards The code that wreaked havoc on my machine did the cleanup with a `removeDir(ApplicationsLocation)`, in other words, `rm -rf /Applications`. Evidently it didn't print exactly what it was doing, it just seemed to hang (fortunately I have an HDD, not an SDD). (And of course I have since reverified all permissions in /Applications.) I reported the bug upstreams, but at the moment any fixing appears to be blocked by a single Qt dev, and a fix isn't likely to be included anytime soon anyway (5.7, maybe). I did include a fix in the QSP patch of my own [ticket:48967 port:qt5-kde], of course. I did run into this problem while developing a port, but I don't think there is much risk with published ports. `port test` probably doesn't run with elevated privileges, and port maintainers should just check any (auto)tests or keep/set `test.run no`. However, anyone using the installed Qt5 SDK should be aware of the risk. -- Ticket URL: <https://trac.macports.org/ticket/49721> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Changes (by mk@…): * owner: macports-tickets@… => mcalhoun@… -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:2> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Changes (by mk@…): * cc: michaelld@… (added) -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:3> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Changes (by mcalhoun@…): * status: new => assigned Comment: Replying to [ticket:49721 rjvbertin@…]:
I reported the bug upstreams, but at the moment any fixing appears to be blocked by a single Qt dev, and a fix isn't likely to be included anytime soon anyway (5.7, maybe). Is there a link on https://bugreports.qt.io?
I did include a fix in the QSP patch of my own [ticket:48967 port:qt5-kde], of course. Could you please say which patch?
-- Ticket URL: <https://trac.macports.org/ticket/49721#comment:4> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): Replying to [comment:4 mcalhoun@…]:
Is there a link on https://bugreports.qt.io?
No, this has been discussed on the mailing lists only.
I did include a fix in the QSP patch of my own [ticket:48967 port:qt5-kde], of course. Could you please say which patch?
It has the evocative name fix_qstandardpaths3.patch . -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:5> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): BTW, there's a companion patch called `fix-qsp_fontlocations.patch` that addresses a related issue in the FontsLocation. That one is separate because I did report it, and it's supposed to be fixed in 5.7 . -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:6> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by mcalhoun@…): For reference: * The issue was first brought up on the [https://mail.kde.org/pipermail /kde-frameworks-devel/2015-November/thread.html#28556 KDE mailing list]. * It then proceeded to the [http://lists.qt- project.org/pipermail/development/2015-November/thread.html#23735 QT mailing list]. * A possible solution can be found in the [https://trac.macports.org/attachment/ticket/48967/qt5-kde.tar.bz2 zip file] of #48967 or the [https://github.com/RJVB/macstrop/blob/master/aqua/qt5-kde/files/fix- qstandardpaths3.patch git repository]. If I understand the problem: * If {{{isTestModeEnabled()}}} returns true, [http://doc.qt.io/qt-5/qstandardpaths.html standard paths] change. * {{{/Library/Application Support}}} becomes {{{~/.qttest/Application Support}}}. * etc. * On OS X, {{{ApplicationsLocation}}} stays {{{/Applications}}} whether {{{isTestModeEnabled()}}} return true or not.[[BR]] * There is a program out there that runs {{{removeDir(ApplicationsLocation)}}} assuming that it is deleting a folder in {{{~/.qttest}}} * This assumption has catastrophic results on OS X. {{{isTestModeEnabled()}}} return true, what is {{{ApplicationsLocation}}} on Windows and Linux that would make {{{removeDir(ApplicationsLocation)}}} safe?[[BR]] Doesn't {{{removeDir(ApplicationsLocation)}}} delete '''ALL''' other application testing data and not just for that particular program?[[BR]] Are you simply not supposed to run two tests at the same time? This sounds like a problem that needs to be addressed either upstream or with the unit test writers.[[BR]] I would suggest submitting a bug or a patch with either group. -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:7> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): Replying to [comment:7 mcalhoun@…]:
* There is a program out there that runs {{{removeDir(ApplicationsLocation)}}} assuming that it is deleting a folder in {{{~/.qttest}}} * This assumption has catastrophic results on OS X.
There could be any number of such (test) programmes. Part of my current routine in writing KF5 ports is to verify all auto-tests for dangerous behaviour, and replace the port test settings with a warning when I get a hit (and KF5 is being built without my QSP patch).
{{{isTestModeEnabled()}}} return true, what is {{{ApplicationsLocation}}} on Windows and Linux that would make {{{removeDir(ApplicationsLocation)}}} safe?[[BR]]
No idea on MS Windows (look at the relevant Qt code...) but on Linux we're talking about `$HOME/.local/share/applications` vs `$HOME/.qttest/share/applications`. Also, ApplicationsLocation doesn't contain the actual applications but only files with metadata pertaining to applications.
Doesn't {{{removeDir(ApplicationsLocation)}}} delete '''ALL''' other application testing data and not just for that particular program?[[BR]] Are you simply not supposed to run two tests at the same time?
Yes, and yes I guess. Of course if you do, and test b removes stuff test a requires, all that happens is that test a fails. You'll then try test a again, and it'll succeed ... and you'll end up understanding that you're seeing expected behaviour.
This sounds like a problem that needs to be addressed either upstream or with the unit test writers.[[BR]] I would suggest submitting a bug or a patch with either group.
As I said before, Qt is aware, and a fix will appear, but probably later rather than sooner. I also don't know if the issue will address the fact that ApplicationsLocation does not have the same meaning across platforms. That is something that will have to be solved independently. Unit test writers are not doing anything wrong, the bug is not theirs, and without documentation saying that ApplicationsLocation should not be used on OS X they have no reason to take that into consideration. The plan is to re-submit my QSP patch to Qt at some future time when it will have had sufficient real-world testing; changes to QStandardPaths as well as the activator principle to toggle QSP from "native" into "XDG- compliant" mode. -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:8> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): Replying to [comment:8 rjvbertin@…]:
(and KF5 is being built without my QSP patch).
In my current approach, the QSP/XDG activator gets linked to all KF5 frameworks if the `+QspXDG` variant is set. - That variant is the default if the activator is detected locally (but can of course be overridden from the commandline, using `-QspXDG`) - This is the case if `port:qt5-kde` is installed - The KF5 frameworks indicate a preference for `port:qt5-kde` - That means that `port:qt5-kde` will be installed when starting from a clean slate like on the build bots. Thus, subports providing individual KF5 frameworks can simply check if +QspXDG is set; if not, `test.cmd` and `test.args` can be set so that a warning is printed. Ports can also use `${qt_bins_dir}/qtpath --testmode --writable-path ApplicationsDir` to check the actual location used, but that is not going to inform whether about QSP's operating mode during the tests. -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:9> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by mcalhoun@…): For such a substantial [https://github.com/RJVB/macstrop/blob/master/aqua/qt5-kde/files/fix- qstandardpaths3.patch patch], I think that the best course of action is to first submit it to [http://codereview.qt-project.org codereview.qt- project.org].[[BR]] If it gets merged upstream, then we can incorporate it our repository. -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:10> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): You don't get it... The patch is indeed substantial, and that's the very reason why it won't be considered upstream until there's real-world evidence to show that it has been tested and works as advertised, without side-effects etc. And once it is incorporated there's no more use to incorporate it in MacPorts ... -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:11> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by mcalhoun@…): Replying to [comment:11 rjvbertin@…]:
You don't get it... I do appreciate you taking the time to patiently explain it.
The patch is indeed substantial, and that's the very reason why it won't be considered upstream until there's real-world evidence to show that it has been tested and works as advertised, without side-effects etc. I am a full supporter of cutting edge experimentation but only in some sort of -devel port.[[BR]] Personally, I use Qt for my work and need at least one non-experimental version around.
And once it is incorporated there's no more use to incorporate it in MacPorts ... Perhaps this is the point of confusion.[[BR]] When I say merged, I mean merged into the git repository that Qt uses for development.[[BR]] I do not mean released.[[BR]] If you look at certain changes ([https://codereview.qt- project.org/#/c/140876/ 140876], [https://codereview.qt-project.org/#/c/125968/ 125968], [https://codereview.qt-project.org/#/c/127759/ 127759] to name just a few), they have been merged but not released.[[BR]] In fact, it may be quite some time (months) before they are released, but since they have been vetted and approved by upstream developers, they are excellent candidates for patchfiles.[[BR]] For those types of changes, there is indeed great use in incorporating them into MacPorts.[[BR]] In fact, Qt will not fully build on El Capitan without at least [https://codereview.qt-project.org/#/c/127759/ one of them].
-- Ticket URL: <https://trac.macports.org/ticket/49721#comment:12> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): Replying to [comment:12 mcalhoun@…]:
I am a full supporter of cutting edge experimentation but only in some sort of -devel port.[[BR]] Personally, I use Qt for my work and need at least one non-experimental version around.
Well, that's just one more argument to have a qt5-kde port, and for the approach I'm taking with the patch in question. If you read my earlier explanation, it doesn't change anything if you don't activate the new code-paths it provides. That is also a prerequisite for getting it accepted upstream, because Apple won't accept applications using XDG- compliant locations in their App Store. Since you mention it: does it matter for your work whether Qt applications use ~/Library/Preferences and ~/Library/Application Support or rather ~/.config and ~/.local/share ? It's the possibility to support both options that makes my patch so complicated (and hard to defend without being able to refer to real-world use cases).
Perhaps this is the point of confusion.[[BR]] When I say merged, I mean merged into the git repository that Qt uses for development.[[BR]] I do not mean released.[[BR]]
Oh, I know that. The point remains that Qt's vetting process is one I only engage in if I'm more or less certain that my proposition will be accepted. And when that proposition isn't changing or intersecting with something else I'm working on, because their "gerrit" is *really* annoying in that case.
If you look at certain changes ([https://codereview.qt- project.org/#/c/140876/ 140876], [https://codereview.qt-project.org/#/c/125968/ 125968], [https://codereview.qt-project.org/#/c/127759/ 127759] to name just a few), they have been merged but not released.[[BR]] In fact, it may be quite some time (months) before they are released, but since they have been vetted and approved by upstream developers, they are excellent candidates for patchfiles.[[BR]] For those types of changes, there is indeed great use in incorporating them into MacPorts.[[BR]] In fact, Qt will not fully build on El Capitan without at least [https://codereview.qt-project.org/#/c/127759/ one of them].
-- Ticket URL: <https://trac.macports.org/ticket/49721#comment:13> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: dangerous bug in Qt5 --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): One more thing: the development version we're talking about is 5.7 . That's *two* "minor" releases away from the current, and in addition the code in question has been rewritten to remove the use of Carbon APIs. In other words, even if my patch gets accepted it will not apply "as is" to older versions. -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:14> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: qt5: some programs' tests try to delete /Applications --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:15> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: qt5: some programs' tests try to delete /Applications --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by rjvbertin@…): You don't think that's a dangerous bug? -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:16> MacPorts <https://www.macports.org/> Ports system for OS X
#49721: qt5: some programs' tests try to delete /Applications --------------------------+------------------------ Reporter: rjvbertin@… | Owner: mcalhoun@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: Resolution: | Keywords: Port: qt5 | --------------------------+------------------------ Comment (by mk@…): +1 I think it is a dangerous bug! -- Ticket URL: <https://trac.macports.org/ticket/49721#comment:17> MacPorts <https://www.macports.org/> Ports system for OS X
participants (1)
-
MacPorts