Skip to content

Commit 50ab07a

Browse files
uemanmarandaneto
andauthored
Fix: Trim Unicode NULL character for Windows support (#420)
* Fix: Trim Unicode Character 'NULL' (U+0000) for Windows * Add PR ID * Rename method and add some explanation * Update flutter/lib/src/default_integrations.dart Co-authored-by: Manoel Aranda Neto <[email protected]> * Add another test Co-authored-by: Manoel Aranda Neto <[email protected]>
1 parent a874f04 commit 50ab07a

File tree

3 files changed

+92
-9
lines changed

3 files changed

+92
-9
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* Fix: Mark `Sentry.currentHub` as deprecated (#406)
88
* Fix: Use name from pubspec.yaml for release if package id is not available (#411)
99
* Feat: `SentryHttpClient` tracks the duration which a request takes and logs failed requests (#414)
10+
* Fix: Trim `\u0000` from Windows package info (#420)
1011

1112
# 5.0.0
1213

flutter/lib/src/default_integrations.dart

+26-8
Original file line numberDiff line numberDiff line change
@@ -366,19 +366,31 @@ class LoadReleaseIntegration extends Integration<SentryFlutterOptions> {
366366
try {
367367
if (options.release == null || options.dist == null) {
368368
final packageInfo = await _packageLoader();
369-
var name = packageInfo.packageName;
369+
var name = _cleanString(packageInfo.packageName);
370370
if (name.isEmpty) {
371371
// Not all platforms have a packageName.
372372
// If no packageName is available, use the appName instead.
373-
name = _cleanAppName(packageInfo.appName);
373+
name = _cleanString(packageInfo.appName);
374+
}
375+
376+
final version = _cleanString(packageInfo.version);
377+
final buildNumber = _cleanString(packageInfo.buildNumber);
378+
379+
var release = name;
380+
if (version.isNotEmpty) {
381+
release = '$release@$version';
382+
}
383+
// At least windows sometimes does not have a buildNumber
384+
if (buildNumber.isNotEmpty) {
385+
release = '$release+$buildNumber';
374386
}
375387

376-
final release =
377-
'$name@${packageInfo.version}+${packageInfo.buildNumber}';
378388
options.logger(SentryLevel.debug, 'release: $release');
379389

380390
options.release = options.release ?? release;
381-
options.dist = options.dist ?? packageInfo.buildNumber;
391+
if (buildNumber.isNotEmpty) {
392+
options.dist = options.dist ?? buildNumber;
393+
}
382394
}
383395
} catch (error) {
384396
options.logger(
@@ -388,15 +400,21 @@ class LoadReleaseIntegration extends Integration<SentryFlutterOptions> {
388400
options.sdk.addIntegration('loadReleaseIntegration');
389401
}
390402

391-
String _cleanAppName(String appName) {
403+
/// This method cleans the given string from characters which should not be
404+
/// used.
405+
/// For example https://docs.sentry.io/platforms/flutter/configuration/releases/#bind-the-version
406+
/// imposes some requirements. Also Windows uses some characters which
407+
/// should not be used.
408+
String _cleanString(String appName) {
392409
// Replace disallowed chars with an underscore '_'
393-
// https://docs.sentry.io/platforms/flutter/configuration/releases/#bind-the-version
394410
return appName
395411
.replaceAll('/', '_')
396412
.replaceAll('\\', '_')
397413
.replaceAll('\t', '_')
398414
.replaceAll('\r\n', '_')
399415
.replaceAll('\r', '_')
400-
.replaceAll('\n', '_');
416+
.replaceAll('\n', '_')
417+
// replace Unicode NULL character with an empty string
418+
.replaceAll('\u{0000}', '');
401419
}
402420
}

flutter/test/default_integrations_test.dart

+65-1
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ void main() {
267267
expect(fixture.options.dist, '789');
268268
});
269269

270-
test('release name does not contain ivalid chars', () async {
270+
test('release name does not contain invalid chars defined by Sentry',
271+
() async {
271272
final loader = () {
272273
return Future.value(PackageInfo(
273274
appName: '\\/sentry\tflutter \r\nfoo\nbar\r',
@@ -283,6 +284,69 @@ void main() {
283284
expect(fixture.options.release, '__sentry_flutter [email protected]+789');
284285
expect(fixture.options.dist, '789');
285286
});
287+
288+
/// See the following issues:
289+
/// - https://github.com/getsentry/sentry-dart/issues/410
290+
/// - https://github.com/fluttercommunity/plus_plugins/issues/182
291+
test('does not send Unicode NULL \\u0000 character in app name or version',
292+
() async {
293+
final loader = () {
294+
return Future.value(PackageInfo(
295+
// As per
296+
// https://api.dart.dev/stable/2.12.4/dart-core/String-class.html
297+
// this is how \u0000 is added to a string in dart
298+
appName: 'sentry_flutter_example\u{0000}',
299+
packageName: '',
300+
version: '1.0.0\u{0000}',
301+
buildNumber: '',
302+
));
303+
};
304+
await fixture
305+
.getIntegration(loader: loader)
306+
.call(MockHub(), fixture.options);
307+
308+
expect(fixture.options.release, '[email protected]');
309+
});
310+
311+
/// See the following issues:
312+
/// - https://github.com/getsentry/sentry-dart/issues/410
313+
/// - https://github.com/fluttercommunity/plus_plugins/issues/182
314+
test(
315+
'does not send Unicode NULL \\u0000 character in package name or build number',
316+
() async {
317+
final loader = () {
318+
return Future.value(PackageInfo(
319+
// As per
320+
// https://api.dart.dev/stable/2.12.4/dart-core/String-class.html
321+
// this is how \u0000 is added to a string in dart
322+
appName: '',
323+
packageName: 'sentry_flutter_example\u{0000}',
324+
version: '',
325+
buildNumber: '123\u{0000}',
326+
));
327+
};
328+
await fixture
329+
.getIntegration(loader: loader)
330+
.call(MockHub(), fixture.options);
331+
332+
expect(fixture.options.release, 'sentry_flutter_example+123');
333+
});
334+
335+
test('dist is null if build number is an empty string', () async {
336+
final loader = () {
337+
return Future.value(PackageInfo(
338+
appName: 'sentry_flutter_example',
339+
packageName: 'a.b.c',
340+
version: '1.0.0',
341+
buildNumber: '',
342+
));
343+
};
344+
await fixture
345+
.getIntegration(loader: loader)
346+
.call(MockHub(), fixture.options);
347+
348+
expect(fixture.options.dist, isNull);
349+
});
286350
});
287351
}
288352

0 commit comments

Comments
 (0)