8000 fix: increase max crash key value length by nornagon · Pull Request #24853 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: increase max crash key value length #24853

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.

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

Merged
merged 1 commit into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/api/crash-reporter.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,16 @@ parameters in a renderer process will not result in those parameters being sent
with crashes that occur in other renderer processes or in the main process.

**Note:** Parameters have limits on the length of the keys and values. Key
names must be no longer than 39 bytes, and values must be no longer than 127
names must be no longer than 39 bytes, and values must be no longer than 20320
bytes. Keys with names longer than the maximum will be silently ignored. Key
values longer than the maximum length will be truncated.

**Note:** On linux values that are longer than 127 bytes will be chunked into
multiple keys, each 127 bytes in length. E.g. `addExtraParameter('foo', 'a'.repeat(130))`
will result in two chunked keys `foo__1` and `foo__2`, the first will contain
the first 127 bytes and the second will contain the remaining 3 bytes. On
your crash reporting backend you should stitch together keys in this format.

### `crashReporter.removeExtraParameter(key)`

* `key` String - Parameter key, must be no longer than 39 bytes.
Expand Down
14 changes: 13 additions & 1 deletion shell/common/crash_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,19 @@ namespace crash_keys {

namespace {

using ExtraCrashKeys = std::deque<crash_reporter::CrashKeyString<127>>;
#if defined(OS_LINUX)
// Breakpad has a flawed system of calculating the number of chunks
// we add 127 bytes to force an extra chunk
constexpr size_t kMaxCrashKeyValueSize = 20479;
#else
constexpr size_t kMaxCrashKeyValueSize = 20320;
#endif

static_assert(kMaxCrashKeyValueSize < crashpad::Annotation::kValueMaxSize,
"max crash key value length above what crashpad supports");

using ExtraCrashKeys =
std::deque<crash_reporter::CrashKeyString<kMaxCrashKeyValueSize>>;
ExtraCrashKeys& GetExtraCrashKeys() {
static base::NoDestructor<ExtraCrashKeys> extra_keys;
return *extra_keys;
Expand Down
17 changes: 14 additions & 3 deletions spec-main/api-crash-reporter-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,19 +251,30 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
});

ifdescribe(!isLinuxOnArm)('extra parameter limits', () => {
it('should truncate extra values longer than 127 characters', async () => {
function stitchLongCrashParam (crash: any, paramKey: string) {
if (crash[paramKey]) return crash[paramKey];
let chunk = 1;
let stitched = '';
while (crash[`${paramKey}__${chunk}`]) {
stitched += crash[`${paramKey}__${chunk}`];
chunk++;
}
return stitched;
}

it('should truncate extra values longer than 5 * 4096 characters', async () => {
const { port, waitForCrash } = await startServer();
const { remotely } = await startRemoteControlApp();
remotely((port: number) => {
require('electron').crashReporter.start({
submitURL: `http://127.0.0.1:${port}`,
ignoreSystemCrashHandler: true,
extra: { 'longParam': 'a'.repeat(130) }
extra: { longParam: 'a'.repeat(100000) }
});
setTimeout(() => process.crash());
}, port);
const crash = await waitForCrash();
expect(crash).to.have.property('longParam', 'a'.repeat(127));
expect(stitchLongCrashParam(crash, 'longParam')).to.have.lengthOf(160 * 127, 'crash should have truncated longParam');
});

it('should omit extra keys with names longer than the maximum', async () => {
Expand Down
0