Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

background-portal: Stop sending app-id quoted #778

Merged
merged 1 commit into from
May 4, 2022
Merged

background-portal: Stop sending app-id quoted #778

merged 1 commit into from
May 4, 2022

Conversation

sophie-h
Copy link
Contributor

Since #696, the arguments sent to background portal implementations
contained the app-id in quoted form. Implementations, in turn, created
invalid autostart files.

@sophie-h
Copy link
Contributor Author

While 28ec93c mentions

First parse the argv so it's unquoted then quote any arguments that need
it. Don't do so in the background portal since the backend handles it.

I think the commit misses excluding the app-id from quoting for the background portal.

@sophie-h sophie-h force-pushed the wip/sophie-h/autostart-appid-escape branch from 3e2e125 to 7f67cac Compare April 19, 2022 01:31
@sophie-h sophie-h changed the title background-portal: Stop sending app-id escaped background-portal: Stop sending app-id quoted Apr 19, 2022
smcv
smcv reviewed Apr 19, 2022
View reviewed changes
@sophie-h
Copy link
Contributor Author

Thanks for fixing the coding style!

@sophie-h sophie-h force-pushed the wip/sophie-h/autostart-appid-escape branch from 804aaf7 to 46ed6ca Compare April 28, 2022 00:26
@sophie-h
Copy link
Contributor Author

@mleeds @GeorgesStavracas You authored/merged the original changes. Maybe you can have a look at this?

As I see it the implications of this bug are that, for example, background processes for scheduled backups are not started. IIRC the autostart file is never rewritten. That means if anyone sets up the app with 1.14.0 to 1.14.3 the autostart file might never get automatically fixed. Therefore it feels somewhat urgent for me to release this fix.

@vchernin vchernin mentioned this pull request May 1, 2022
Closed
GeorgesStavracas
GeorgesStavracas approved these changes May 4, 2022
View reviewed changes
Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash the second commit with the first one. LGTM otherwise.

@sophie-h sophie-h force-pushed the wip/sophie-h/autostart-appid-escape branch from 46ed6ca to 4f94051 Compare May 4, 2022 15:38
@sophie-h
Copy link
Contributor Author

sophie-h commented May 4, 2022

Please squash the second commit with the first one. LGTM otherwise.

Done :)

@sophie-h @GeorgesStavracas
background-portal: Stop sending app-id quoted
a6faf19 
Since flatpak#696, the arguments sent to background portal implementations
contained the app-id in quoted form. Implementations, in turn, created
invalid autostart files.
@GeorgesStavracas GeorgesStavracas force-pushed the wip/sophie-h/autostart-appid-escape branch from 4f94051 to a6faf19 Compare May 4, 2022 15:38
@GeorgesStavracas GeorgesStavracas merged commit d26e352 into flatpak:main May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smcv smcv smcv left review comments

@GeorgesStavracas GeorgesStavracas GeorgesStavracas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sophie-h @smcv @GeorgesStavracas