diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 3e3fcf4f1f..ee8116642f 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -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. diff --git a/shell/common/crash_keys.cc b/shell/common/crash_keys.cc index a5d92497c9..d71ce04487 100644 --- a/shell/common/crash_keys.cc +++ b/shell/common/crash_keys.cc @@ -24,7 +24,19 @@ namespace crash_keys { namespace { -using ExtraCrashKeys = std::deque>; +#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>; ExtraCrashKeys& GetExtraCrashKeys() { static base::NoDestructor extra_keys; return *extra_keys; diff --git a/spec-main/api-crash-reporter-spec.ts b/spec-main/api-crash-reporter-spec.ts index d1fd03508a..1d7ab98ab5 100644 --- a/spec-main/api-crash-reporter-spec.ts +++ b/spec-main/api-crash-reporter-spec.ts @@ -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 () => {