Skip to content
Merged
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
55 changes: 36 additions & 19 deletions lib/utils/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ abstract class Utils {
onError: onError,
);
} else {
final newImageUrl = _removeTbImagePrefix(imageUrl);
if (_isImageResourceUrl(newImageUrl)) {
final resolvedImageUrl = _removeTbImagePrefix(imageUrl);
if (_isImageResourceUrl(resolvedImageUrl)) {
final jwtToken = tbClient.getJwtToken();
if (jwtToken == null) {
return _onErrorImage(
Expand All @@ -98,7 +98,7 @@ abstract class Utils {
onError: onError,
);
}
final parts = newImageUrl.split('/');
final parts = resolvedImageUrl.split('/');
final key = parts[parts.length - 1];
parts[parts.length - 1] = Uri.encodeComponent(key);
final encodedUrl = parts.join('/');
Expand All @@ -115,29 +115,31 @@ abstract class Utils {
semanticLabel: semanticLabel,
onError: onError,
);
} else if (_isBase64DataImageUrl(imageUrl)) {
} else if (_isBase64DataImageUrl(resolvedImageUrl)) {
return _imageFromBase64(
context,
imageUrl,
color: color,
width: width,
height: height,
semanticLabel: semanticLabel,
onError: onError,
);
} else if (_isValidUrl(imageUrl)) {
return _networkImage(
context,
imageUrl,
resolvedImageUrl,
color: color,
width: width,
height: height,
semanticLabel: semanticLabel,
onError: onError,
);
} else {
return _onErrorImage(
final imageLink = _resolveNetworkImageLink(resolvedImageUrl);
if (imageLink == null) {
return _onErrorImage(
context,
color: color,
width: width,
height: height,
semanticLabel: semanticLabel,
onError: onError,
);
}
return _networkImage(
context,
imageLink,
color: color,
width: width,
height: height,
Expand Down Expand Up @@ -287,15 +289,30 @@ abstract class Utils {
}

static String _removeTbImagePrefix(String url) {
return url.replaceFirst(_tbImagePrefix, '');
return url.startsWith(_tbImagePrefix)
? url.substring(_tbImagePrefix.length)
: url;
}

static bool _isImageResourceUrl(String url) {
return _imagesUrlRegexp.hasMatch(url);
}

static bool _isValidUrl(String url) {
return Uri.tryParse(url) != null;
/// HTTP(S) links are fetched as they are. Root-relative links, such as an
/// image public link, are resolved against the active endpoint. Anything
/// else has no meaningful target and is rendered as a missing image.
static String? _resolveNetworkImageLink(String url) {
final uri = Uri.tryParse(url);
if (uri == null) {
return null;
}
if (uri.scheme == 'http' || uri.scheme == 'https') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only nit I have on the new helper: the comment above says "Absolute links are fetched as they are", but this gate passes only http/httpsftp:, file: and any custom scheme fall through to the final return null alongside genuinely unresolvable input. That's the behaviour you want, the sentence just makes it read like an oversight. "HTTP(S) links are fetched as they are" would make the restriction look as deliberate as it is.

For the record I did go looking for a hole in this gate and didn't find one worth acting on: uri.scheme is normalised, so HTTPS://… is handled, and the one input that slips through is https:example.com/pic.png (the missing-// typo), which reaches Image.network with an empty host. It renders the placeholder either way, before and after this PR, so guarding it would add a line for no observable change — noting it only so it doesn't get re-raised later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1eb4637 — reworded to "HTTP(S) links are fetched as they are" so the scheme gate reads as intentional.

return url;
}
if (!uri.hasScheme && url.startsWith('/')) {
return getIt<IEndpointService>().getCachedEndpoint() + url;
}
return null;
}

static double degreesToRadians(double degrees) {
Expand Down