Skip to content

Commit

Permalink
fix: do not propagate GDK_BACKEND env variable to subproc
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere authored and bavulapati committed Feb 17, 2022
1 parent c75ec2e commit 15af724
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 0 deletions.
16 changes: 16 additions & 0 deletions shell/browser/electron_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,24 @@ void ElectronBrowserMainParts::PostDestroyThreads() {
fake_browser_process_->PostDestroyThreads();
}

#if BUILDFLAG(IS_LINUX)
// static
absl::optional<std::string>& ElectronBrowserMainParts::GetGDKBackend() {
static absl::optional<std::string> gdk_backend;
return gdk_backend;
}
#endif

void ElectronBrowserMainParts::ToolkitInitialized() {
#if BUILDFLAG(IS_LINUX)
// This is set by Chromium here:
// https://chromium-review.googlesource.com/c/chromium/src/+/2586184
// and can detrimentally affect external app behaviors, so we want to
// check if the user has set it so we can use it later.
std::string backend;
if (base::Environment::Create()->GetVar("GDK_BACKEND", &backend))
GetGDKBackend() = backend;

auto linux_ui = BuildGtkUi();
linux_ui->Initialize();
DCHECK(ui::LinuxInputMethodContextFactory::instance());
Expand Down
5 changes: 5 additions & 0 deletions shell/browser/electron_browser_main_parts.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ class ElectronBrowserMainParts : public content::BrowserMainParts {
Browser* browser() { return browser_.get(); }
BrowserProcessImpl* browser_process() { return fake_browser_process_.get(); }

#if BUILDFLAG(IS_LINUX)
// Used by platform_util to set GDK_BACKEND.
static absl::optional<std::string>& GetGDKBackend();
#endif

protected:
// content::BrowserMainParts:
int PreEarlyInitialization() override;
Expand Down
13 changes: 13 additions & 0 deletions shell/common/platform_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "dbus/bus.h"
#include "dbus/message.h"
#include "dbus/object_proxy.h"
#include "shell/browser/electron_browser_main_parts.h"
#include "shell/common/platform_util_internal.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gtk/gtk_util.h"
Expand Down Expand Up @@ -275,6 +276,18 @@ bool XDGUtil(const std::vector<std::string>& argv,
// bring up a new terminal if necessary. See "man mailcap".
options.environment["MM_NOTTTY"] = "1";

// If the user sets a GDK_BACKEND value of their own, use that,
// otherwise unset it because Chromium is setting GDK_BACKEND
// during GTK initialization and we want to respect user preference.
// Setting values in EnvironmentMap to an empty-string
// will make sure that they get unset from the environment via
// AlterEnvironment().
const absl::optional<std::string>& gdk_backend =
electron::ElectronBrowserMainParts::GetGDKBackend();
options.environment["GDK_BACKEND"] = gdk_backend.has_value()
? gdk_backend.value().c_str()
: base::NativeEnvironmentString();

base::Process process = base::LaunchProcess(argv, options);
if (!process.IsValid())
return false;
Expand Down
73 changes: 73 additions & 0 deletions spec-main/chromium-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { promisify } from 'util';
import { ifit, ifdescribe, delay, defer } from './spec-helpers';
import { AddressInfo } from 'net';
import { PipeTransport } from './pipe-transport';
const timersPromises = require('timers/promises');

const features = process._linkedBinding('electron_common_features');

Expand Down Expand Up @@ -364,6 +365,78 @@ describe('web security', () => {
});
});

ifdescribe(process.platform === 'linux')('subprocesses', () => {
let appProcess: ChildProcess.ChildProcessWithoutNullStreams | undefined;
const appPath = path.join(fixturesPath, 'api', 'gdk-backend-check');
beforeEach(() => {
ChildProcess.execFileSync(path.join(appPath, 'before-test.sh'), { cwd: appPath, encoding: 'utf8' });
});
afterEach(() => {
ChildProcess.execFileSync(path.join(appPath, 'after-test.sh'), { cwd: appPath, encoding: 'utf8' });
if (appProcess && !appProcess.killed) {
appProcess.kill();
appProcess = undefined;
}
});

it('does not propagate GDK_BACKEND', async () => {
appProcess = ChildProcess.spawn(process.execPath, [path.join(appPath, 'main.js')], { env: { ...process.env, GDK_BACKEND: '' } });

let output = '';
appProcess.stdout.on('data', (data) => { output += data; });
let stderr = '';
appProcess.stderr.on('data', (data) => { stderr += data; });

const [code, signal] = await emittedOnce(appProcess, 'exit');
if (code !== 0) {
throw new Error(`Process exited with code "${code}" signal "${signal}" output "${output}" stderr "${stderr}"`);
}

for await (const startTime of timersPromises.setInterval(10, Date.now())) {
if (fs.existsSync('/tmp/groot-says')) {
let gdkBackend = fs.readFileSync('/tmp/groot-says', 'utf8');
gdkBackend = gdkBackend.trim();

expect(gdkBackend).to.be.empty();
break;
}
const now = Date.now();
if ((now - startTime) > 5000) {
throw new Error('The gdk backend test failed timing out on waiting for the file');
}
}
});

it('successfully honors GDK_BACKEND set in the subproc', async () => {
appProcess = ChildProcess.spawn(process.execPath, [path.join(appPath, 'main.js')], { env: { ...process.env, GDK_BACKEND: 'groot' } });

let output = '';
appProcess.stdout.on('data', (data) => { output += data; });
let stderr = '';
appProcess.stderr.on('data', (data) => { stderr += data; });

const [code, signal] = await emittedOnce(appProcess, 'exit');
if (code !== 0) {
throw new Error(`Process exited with code "${code}" signal "${signal}" output "${output}" stderr "${stderr}"`);
}

for await (const startTime of timersPromises.setInterval(10, Date.now())) {
if (fs.existsSync('/tmp/groot-says')) {
let gdkBackend = fs.readFileSync('/tmp/groot-says', 'utf8');
gdkBackend = gdkBackend.trim();

expect(gdkBackend).to.not.be.empty();
expect(gdkBackend).to.equal('groot');
break;
}
const now = Date.now();
if ((now - startTime) > 5000) {
throw new Error('The gdk backend test failed timing out on waiting for the file');
}
}
});
});

describe('command line switches', () => {
let appProcess: ChildProcess.ChildProcessWithoutNullStreams | undefined;
afterEach(() => {
Expand Down
13 changes: 13 additions & 0 deletions spec/fixtures/api/gdk-backend-check/after-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/sh

set -o errexit

rm -f /tmp/groot-says
rm -f /tmp/groot
rm -f ~/.local/share/applications/groot.desktop

update-desktop-database ~/.local/share/applications

rm -f ~/.local/share/mime/packages/groot.xml

update-mime-database ~/.local/share/mime
24 changes: 24 additions & 0 deletions spec/fixtures/api/gdk-backend-check/before-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/sh

set -o errexit

cleanup () {
exec "./after-test.sh"
}

trap cleanup HUP INT

# Install the application Groot
cp ./groot /tmp/

# Register Groot as desktop application
cp ./groot.desktop ~/.local/share/applications

# Update database of desktop entries
update-desktop-database ~/.local/share/applications

# Associate 'application/x-groot' mime type with '*.groot' files
cp ./groot.xml ~/.local/share/mime/packages/

# update the MIME database
update-mime-database ~/.local/share/mime
2 changes: 2 additions & 0 deletions spec/fixtures/api/gdk-backend-check/groot
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/sh
echo $GDK_BACKEND > /tmp/groot-says
5 changes: 5 additions & 0 deletions spec/fixtures/api/gdk-backend-check/groot.desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[Desktop Entry]
Type=Application
Exec=/tmp/groot
Name=Groot
MimeType=application/x-groot
7 changes: 7 additions & 0 deletions spec/fixtures/api/gdk-backend-check/groot.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<mime-info xmlns="http://www.freedesktop.org/standards/shared-mime-info">
<mime-type type="application/x-groot">
<comment>Groot File</comment>
<glob weight="100" pattern="*.groot"/>
</mime-type>
</mime-info>
1 change: 1 addition & 0 deletions spec/fixtures/api/gdk-backend-check/hello.groot
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#!/bin/sh
7 changes: 7 additions & 0 deletions spec/fixtures/api/gdk-backend-check/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const { app, shell } = require('electron');
const path = require('path');

app.whenReady().then(async () => {
await shell.openPath(path.join(__dirname, 'hello.groot'));
app.quit();
});

0 comments on commit 15af724

Please sign in to comment.