From 91e61af8fa5e8ce7dca67b36535eb8e70d9dd5cd Mon Sep 17 00:00:00 2001 From: tank <322465767+neo22neo@users.noreply.github.com> Date: Thu, 3 Sep 2026 21:27:44 -0400 Subject: [PATCH 1/2] Validate Mbin community names when creating a community Mbin rejects magazine names that fall outside /^[a-zA-Z0-9_]{2,25}$/, but the create-community form only disabled submit on an empty name, so an invalid name failed server-side with no guidance. - Add mbin_community_name.dart with pure, tested helpers to validate a name, describe why it is invalid, and derive a sanitized suggestion. - Give the shared TextEditor helperText/errorText support. - In CommunityOwnerPanelGeneral (creation only), on Mbin: show the rule as helper text, an inline error while the name is invalid, a one-tap "Use suggestion" action, and keep submit disabled until the name is valid. Lemmy/PieFed behaviour is unchanged. - Add flutter_test dev dependency and focused unit tests for the helpers. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N7b7QotawaBJQ4isQKd14x --- lib/l10n/app_en.arb | 26 +++++ .../explore/community_owner_panel.dart | 67 +++++++++++-- lib/src/utils/mbin_community_name.dart | 61 ++++++++++++ lib/src/widgets/text_editor.dart | 8 ++ pubspec.yaml | 4 + test/utils/mbin_community_name_test.dart | 96 +++++++++++++++++++ 6 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 lib/src/utils/mbin_community_name.dart create mode 100644 test/utils/mbin_community_name_test.dart diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 55633f45..eb7aec16 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -537,6 +537,32 @@ "create_link_invalid": "Url is invalid", "create_microblog": "Microblog", "create_community": "Community", + "community_nameMbinHelp": "2–25 characters. Letters, numbers and underscores only.", + "community_nameInvalidCharacters": "Only letters, numbers and underscores are allowed.", + "community_nameTooShort": "Name must be at least {count} characters.", + "@community_nameTooShort": { + "placeholders": { + "count": { + "type": "int" + } + } + }, + "community_nameTooLong": "Name must be at most {count} characters.", + "@community_nameTooLong": { + "placeholders": { + "count": { + "type": "int" + } + } + }, + "community_nameUseSuggestion": "Use “{name}”", + "@community_nameUseSuggestion": { + "placeholders": { + "name": { + "type": "String" + } + } + }, "microblog_communityHelperText": "Defaults to the 'random' community for microblogs since that's where Mbin stores uncatagorized microblogs.", "selectCommunity": "Select a community ...", "title": "Title", diff --git a/lib/src/screens/explore/community_owner_panel.dart b/lib/src/screens/explore/community_owner_panel.dart index 2c1e5833..0e43f5f7 100644 --- a/lib/src/screens/explore/community_owner_panel.dart +++ b/lib/src/screens/explore/community_owner_panel.dart @@ -1,9 +1,11 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:interstellar/src/controller/controller.dart'; +import 'package:interstellar/src/controller/server.dart'; import 'package:interstellar/src/models/community.dart'; import 'package:interstellar/src/models/user.dart'; import 'package:interstellar/src/screens/explore/user_item.dart'; +import 'package:interstellar/src/utils/mbin_community_name.dart'; import 'package:interstellar/src/utils/utils.dart'; import 'package:interstellar/src/widgets/loading_button.dart'; import 'package:interstellar/src/widgets/markdown/drafts_controller.dart'; @@ -112,23 +114,75 @@ class _CommunityOwnerPanelGeneralState widget.data?.isPostingRestrictedToMods ?? false; } + String? _mbinNameError(BuildContext context, MbinCommunityNameIssue? issue) { + switch (issue) { + case MbinCommunityNameIssue.invalidCharacters: + return l(context).community_nameInvalidCharacters; + case MbinCommunityNameIssue.tooShort: + return l(context).community_nameTooShort(mbinCommunityNameMinLength); + case MbinCommunityNameIssue.tooLong: + return l(context).community_nameTooLong(mbinCommunityNameMaxLength); + case null: + return null; + } + } + @override Widget build(BuildContext context) { final descriptionDraftController = context.watch().auto( 'community:description${widget.data == null ? '' : ':${widget.data}'}', ); + final isCreating = widget.data == null; + // Mbin is the only backend that rejects names outside of + // /^[a-zA-Z0-9_]{2,25}$/, so only validate/suggest for it. The name field + // itself is only shown while creating; edits never touch the name. + final enforceMbinName = + isCreating && + context.watch().serverSoftware == ServerSoftware.mbin; + + final name = _nameController.text; + final mbinNameIssue = enforceMbinName ? mbinCommunityNameIssue(name) : null; + final mbinNameSuggestion = enforceMbinName + ? suggestMbinCommunityName(name) + : null; + final nameInvalidForMbin = + enforceMbinName && !isValidMbinCommunityName(name); + final mbinNameErrorText = _mbinNameError(context, mbinNameIssue); + return ListView( padding: const EdgeInsets.all(16), children: [ - if (widget.data == null) + if (isCreating) Padding( padding: const EdgeInsets.symmetric(vertical: 16), - child: TextEditor( - _nameController, - label: 'Name', - onChanged: (_) => setState(() {}), - maxLength: 25, + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + TextEditor( + _nameController, + label: 'Name', + onChanged: (_) => setState(() {}), + maxLength: enforceMbinName ? mbinCommunityNameMaxLength : 25, + helperText: enforceMbinName + ? l(context).community_nameMbinHelp + : null, + errorText: mbinNameErrorText, + ), + if (mbinNameSuggestion case final suggestion?) + Align( + alignment: AlignmentDirectional.centerStart, + child: TextButton.icon( + onPressed: () => setState(() { + _nameController.text = suggestion; + }), + icon: const Icon(Symbols.auto_fix_high_rounded), + label: Text( + l(context).community_nameUseSuggestion(suggestion), + ), + ), + ), + ], ), ), Padding( @@ -180,6 +234,7 @@ class _CommunityOwnerPanelGeneralState child: LoadingFilledButton( onPressed: _nameController.text.isEmpty || + nameInvalidForMbin || _titleController.text.isEmpty || (_titleController.text == widget.data?.title && _descriptionController.text == diff --git a/lib/src/utils/mbin_community_name.dart b/lib/src/utils/mbin_community_name.dart new file mode 100644 index 00000000..fea8e813 --- /dev/null +++ b/lib/src/utils/mbin_community_name.dart @@ -0,0 +1,61 @@ +/// Helpers for validating and repairing Mbin magazine (community) names. +/// +/// Mbin restricts magazine names to 2-25 characters consisting only of +/// letters, digits and underscores (`RegPatterns::MAGAZINE_NAME` / +/// `/^[a-zA-Z0-9_]{2,25}$/` upstream). Lemmy and PieFed use different rules, +/// so callers should only apply these checks when talking to an Mbin server. +library; + +const int mbinCommunityNameMinLength = 2; +const int mbinCommunityNameMaxLength = 25; + +final RegExp _mbinCommunityNameRegExp = RegExp(r'^[a-zA-Z0-9_]{2,25}$'); +final RegExp _mbinCommunityNameInvalidChars = RegExp(r'[^a-zA-Z0-9_]'); + +/// Whether [name] is a valid Mbin magazine name that can be submitted as-is. +bool isValidMbinCommunityName(String name) => + _mbinCommunityNameRegExp.hasMatch(name); + +/// The reason [name] is not a valid Mbin magazine name, or `null` when it is +/// valid (or still empty, which is treated as "not entered yet"). +MbinCommunityNameIssue? mbinCommunityNameIssue(String name) { + if (name.isEmpty) return null; + if (_mbinCommunityNameInvalidChars.hasMatch(name)) { + return MbinCommunityNameIssue.invalidCharacters; + } + if (name.length < mbinCommunityNameMinLength) { + return MbinCommunityNameIssue.tooShort; + } + if (name.length > mbinCommunityNameMaxLength) { + return MbinCommunityNameIssue.tooLong; + } + return null; +} + +enum MbinCommunityNameIssue { invalidCharacters, tooShort, tooLong } + +/// A best-effort valid name derived from [name], or `null` when nothing +/// usable can be salvaged (e.g. the input has no letters/digits at all) or +/// when [name] is already valid. +String? suggestMbinCommunityName(String name) { + if (isValidMbinCommunityName(name)) return null; + + // Replace every run of unsupported characters (whitespace, punctuation, + // accented letters, ...) with a single underscore, then tidy up the + // underscores so the result reads naturally. + var suggestion = name + .replaceAll(_mbinCommunityNameInvalidChars, '_') + .replaceAll(RegExp(r'_+'), '_') + .replaceAll(RegExp(r'^_+|_+$'), ''); + + if (suggestion.length > mbinCommunityNameMaxLength) { + suggestion = suggestion + .substring(0, mbinCommunityNameMaxLength) + .replaceAll(RegExp(r'_+$'), ''); + } + + if (suggestion.length < mbinCommunityNameMinLength) return null; + if (suggestion == name) return null; + + return suggestion; +} diff --git a/lib/src/widgets/text_editor.dart b/lib/src/widgets/text_editor.dart index b4f857f7..b593eb4d 100644 --- a/lib/src/widgets/text_editor.dart +++ b/lib/src/widgets/text_editor.dart @@ -6,6 +6,8 @@ class TextEditor extends StatelessWidget { this.keyboardType, this.label, this.hint, + this.helperText, + this.errorText, this.onChanged, this.enabled, this.maxLength, @@ -17,6 +19,8 @@ class TextEditor extends StatelessWidget { final TextInputType? keyboardType; final String? label; final String? hint; + final String? helperText; + final String? errorText; final void Function(String)? onChanged; final bool? enabled; final int? maxLength; @@ -31,6 +35,10 @@ class TextEditor extends StatelessWidget { border: const OutlineInputBorder(), labelText: label, hintText: hint, + helperText: helperText, + helperMaxLines: 3, + errorText: errorText, + errorMaxLines: 3, ), onChanged: onChanged, enabled: enabled, diff --git a/pubspec.yaml b/pubspec.yaml index 5db6b50e..3c275202 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -79,6 +79,10 @@ dependencies: unifiedpush_platform_interface: ^4.0.0 unifiedpush_storage_interface: ^1.0.0 +dev_dependencies: + flutter_test: + sdk: flutter + # Needed for 16kb page, remove once this pr (https://github.com/google/webcrypto.dart/pull/238) is merged and the next version is released. dependency_overrides: webcrypto: diff --git a/test/utils/mbin_community_name_test.dart b/test/utils/mbin_community_name_test.dart new file mode 100644 index 00000000..23d4157e --- /dev/null +++ b/test/utils/mbin_community_name_test.dart @@ -0,0 +1,96 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:interstellar/src/utils/mbin_community_name.dart'; + +void main() { + group('isValidMbinCommunityName', () { + test('accepts letters, digits and underscores within 2-25 chars', () { + expect(isValidMbinCommunityName('ab'), isTrue); + expect(isValidMbinCommunityName('under_score'), isTrue); + expect(isValidMbinCommunityName('Mixed_Case_123'), isTrue); + expect(isValidMbinCommunityName('a' * 25), isTrue); + }); + + test('rejects empty, too short and too long names', () { + expect(isValidMbinCommunityName(''), isFalse); + expect(isValidMbinCommunityName('a'), isFalse); + expect(isValidMbinCommunityName('a' * 26), isFalse); + }); + + test('rejects unsupported characters', () { + expect(isValidMbinCommunityName('with space'), isFalse); + expect(isValidMbinCommunityName('with-hyphen'), isFalse); + expect(isValidMbinCommunityName('dot.separated'), isFalse); + expect(isValidMbinCommunityName('accenté'), isFalse); + }); + }); + + group('mbinCommunityNameIssue', () { + const invalid = MbinCommunityNameIssue.invalidCharacters; + const tooShort = MbinCommunityNameIssue.tooShort; + const tooLong = MbinCommunityNameIssue.tooLong; + + test('returns null for an empty (not yet entered) name', () { + expect(mbinCommunityNameIssue(''), isNull); + }); + + test('returns null for a valid name', () { + expect(mbinCommunityNameIssue('valid_Name_123'), isNull); + }); + + test('reports invalid characters ahead of length problems', () { + expect(mbinCommunityNameIssue('hello world'), invalid); + expect(mbinCommunityNameIssue('!'), invalid); + }); + + test('reports names that are too short', () { + expect(mbinCommunityNameIssue('a'), tooShort); + }); + + test('reports names longer than 25 characters', () { + expect(mbinCommunityNameIssue('a' * 26), tooLong); + }); + }); + + group('suggestMbinCommunityName', () { + test('returns null when the name is already valid', () { + expect(suggestMbinCommunityName('already_valid'), isNull); + }); + + test('replaces unsupported runs with a single underscore', () { + final result = suggestMbinCommunityName('My Cool Community!'); + expect(result, 'My_Cool_Community'); + expect(suggestMbinCommunityName('a...b---c'), 'a_b_c'); + }); + + test('trims leading and trailing underscores', () { + expect(suggestMbinCommunityName(' hello!! '), 'hello'); + }); + + test('truncates to 25 characters without a trailing underscore', () { + final s = suggestMbinCommunityName('abcdefghijklmnopqrstuvwx yz'); + expect(s, 'abcdefghijklmnopqrstuvwx'); + expect(isValidMbinCommunityName(s!), isTrue); + }); + + test('returns null when nothing usable can be salvaged', () { + expect(suggestMbinCommunityName('a'), isNull); + expect(suggestMbinCommunityName(' '), isNull); + expect(suggestMbinCommunityName('日本語'), isNull); + }); + + test('always produces a valid name when it returns one', () { + const inputs = [ + 'hello world', + 'Trailing punctuation???', + '***leading', + 'lots of spaces', + 'cafe-society #2', + ]; + for (final input in inputs) { + final suggestion = suggestMbinCommunityName(input); + if (suggestion == null) continue; + expect(isValidMbinCommunityName(suggestion), isTrue, reason: input); + } + }); + }); +} From 8c710891b6a23e84583882eb8eae1d5ea547b364 Mon Sep 17 00:00:00 2001 From: tank <322465767+neo22neo@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:48:47 -0400 Subject: [PATCH 2/2] Render server validation errors as form errors Mbin rejects some community names for reasons the client cannot predict (already taken, reserved, instance policy) with a 400 whose RFC 7807-style body carries a human-readable `detail`. `checkResponseSuccess` embedded that whole JSON body in an `http.ClientException` message, and the create screen had no try/catch, so it surfaced through the global handler as a snackbar full of raw JSON. - Add `ServerErrorException`, thrown by `checkResponseSuccess` when an error body is a JSON object with a string `title`/`detail`. Its `toString()` returns `detail`/`title`, so every existing generic `catch (e)` renders a clean message instead of raw JSON. Non-structured bodies (Lemmy/PieFed `{"error": ...}`, HTML, empty) still throw `ClientException` unchanged, and no call site matches that type. - Community create screen: catch `ServerErrorException` from `create()`; a 400 carrying a `detail` is shown on the Name field through the existing `errorText` plumbing and swallowed, so no snackbar fires. Anything else is rethrown to the existing generic handling. - The message is tracked alongside the name it was returned for, so it clears itself on any edit, including the suggestion button setting the controller text directly. This is a backstop behind the client-side validation, which already blocks predictable malformed names before submit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015icyWnYYmR7J8oSHocta6U --- lib/src/api/client.dart | 69 +++++++++++++++++++ .../explore/community_owner_panel.dart | 69 ++++++++++++++----- 2 files changed, 120 insertions(+), 18 deletions(-) diff --git a/lib/src/api/client.dart b/lib/src/api/client.dart index 8e927857..8de4534e 100644 --- a/lib/src/api/client.dart +++ b/lib/src/api/client.dart @@ -26,6 +26,40 @@ class RestrictedAuthException implements Exception { } } +/// Thrown when the server responds with an error status and a structured +/// problem body (RFC 7807 / RFC 2616-style JSON), e.g. +/// `{"type": ..., "title": ..., "status": 400, "detail": "..."}`. +/// +/// Callers that know a given failure is really a validation problem can catch +/// this and surface [detail] as a friendly, inline message. Everything else can +/// keep relying on [toString], which stays human-readable. +class ServerErrorException implements Exception { + ServerErrorException({ + required this.statusCode, + required this.uri, + required this.rawBody, + this.title, + this.detail, + }); + + final int statusCode; + + final Uri uri; + + /// Raw response body, kept for logging / debugging. + final String rawBody; + + /// Short summary of the error, if the server provided one. + final String? title; + + /// Human-readable explanation of the error, suitable for showing to users. + final String? detail; + + @override + String toString() => + detail ?? title ?? 'Request failed with status $statusCode: $rawBody'; +} + class ServerClient { ServerClient({ required this.httpClient, @@ -198,6 +232,12 @@ class ServerClient { throw RestrictedAuthException(response.body, url); } + // Prefer a structured problem body (e.g. `{"title": ..., "detail": ...}`) + // so callers can show `detail` as a friendly, inline message instead of a + // raw JSON blob. + final structured = _tryParseServerError(url, response); + if (structured != null) throw structured; + var message = 'Request failed with status ${response.statusCode}'; if (response.reasonPhrase != null) { @@ -210,6 +250,35 @@ class ServerClient { throw http.ClientException(message, url); } + + static ServerErrorException? _tryParseServerError( + Uri url, + http.Response response, + ) { + if (response.body.isEmpty) return null; + + try { + final decoded = jsonDecode(utf8.decode(response.bodyBytes)); + if (decoded is! Map) return null; + + final title = decoded['title']; + final detail = decoded['detail']; + + // Only treat this as a structured error if it actually carries a + // human-readable message; otherwise fall back to the generic exception. + if (title is! String && detail is! String) return null; + + return ServerErrorException( + statusCode: response.statusCode, + uri: url, + rawBody: response.body, + title: title is String ? title : null, + detail: detail is String ? detail : null, + ); + } catch (_) { + return null; + } + } } extension BodyJson on http.Response { diff --git a/lib/src/screens/explore/community_owner_panel.dart b/lib/src/screens/explore/community_owner_panel.dart index 0e43f5f7..40c4cb18 100644 --- a/lib/src/screens/explore/community_owner_panel.dart +++ b/lib/src/screens/explore/community_owner_panel.dart @@ -1,5 +1,6 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; +import 'package:interstellar/src/api/client.dart'; import 'package:interstellar/src/controller/controller.dart'; import 'package:interstellar/src/controller/server.dart'; import 'package:interstellar/src/models/community.dart'; @@ -101,6 +102,13 @@ class _CommunityOwnerPanelGeneralState late bool _isAdult; late bool _isPostingRestrictedToMods; + /// Name validation message returned by the server, and the name it was + /// returned for. It is only shown while the field still holds that name, so + /// it clears itself on any edit, including the suggestion button setting the + /// controller text directly. + String? _nameServerError; + String? _nameServerErrorFor; + @override void initState() { super.initState(); @@ -149,6 +157,9 @@ class _CommunityOwnerPanelGeneralState final nameInvalidForMbin = enforceMbinName && !isValidMbinCommunityName(name); final mbinNameErrorText = _mbinNameError(context, mbinNameIssue); + final serverNameError = _nameServerErrorFor == name + ? _nameServerError + : null; return ListView( padding: const EdgeInsets.all(16), @@ -167,7 +178,7 @@ class _CommunityOwnerPanelGeneralState helperText: enforceMbinName ? l(context).community_nameMbinHelp : null, - errorText: mbinNameErrorText, + errorText: serverNameError ?? mbinNameErrorText, ), if (mbinNameSuggestion case final suggestion?) Align( @@ -245,23 +256,45 @@ class _CommunityOwnerPanelGeneralState ? null : () async { final ac = context.read(); - final result = widget.data == null - ? await ac.api.communityModeration.create( - name: _nameController.text, - title: _titleController.text, - description: _descriptionController.text, - isAdult: _isAdult, - isPostingRestrictedToMods: - _isPostingRestrictedToMods, - ) - : await ac.api.communityModeration.edit( - widget.data!.id, - title: _titleController.text, - description: _descriptionController.text, - isAdult: _isAdult, - isPostingRestrictedToMods: - _isPostingRestrictedToMods, - ); + + if (isCreating) { + final DetailedCommunityModel result; + try { + result = await ac.api.communityModeration.create( + name: _nameController.text, + title: _titleController.text, + description: _descriptionController.text, + isAdult: _isAdult, + isPostingRestrictedToMods: _isPostingRestrictedToMods, + ); + } on ServerErrorException catch (e) { + // Name problems only the server can know about (already + // taken, reserved, instance policy) come back as a 400 + // with a human-readable detail. Show it on the Name + // field instead of letting the global snackbar fire. + if (e.statusCode == 400 && e.detail != null) { + setState(() { + _nameServerError = e.detail; + _nameServerErrorFor = _nameController.text; + }); + return; + } + rethrow; + } + + await descriptionDraftController.discard(); + + widget.onUpdate(result); + return; + } + + final result = await ac.api.communityModeration.edit( + widget.data!.id, + title: _titleController.text, + description: _descriptionController.text, + isAdult: _isAdult, + isPostingRestrictedToMods: _isPostingRestrictedToMods, + ); await descriptionDraftController.discard();