Skip to content

Commit

Permalink
android notif: Parse sender_avatar_url at crunchy shell.
Browse files Browse the repository at this point in the history
Since at least server v1.6.0, we've never sent a `base_url` --
so the getter `getBaseURL` has never returned anything but `null`.

So, first: hard-wire that to `null`, and constant-fold it through
several layers where a `baseURL` value was passed.

Doing so, we learn in `addHost` that if we'd ever had a
`sender_avatar_url` which didn't start with `http`, we would have
blown up with a NullPointerException on trying to use `baseURL`
to fix it.  This seems to be meant as a way of testing for a
relative URL reference, vs. a real absolute URL (though it's an
awfully buggy one.)

So, pull that check up into the "crunchy shell" of parsing, and
handle it appropriately.  Also replace the check's logic with a
non-buggy version: check that the URL parses as a real, absolute URL.

A question this raises is: how likely was it that we'd get one of
these crash-causing FCM messages (with `sender_avatar_url` not
starting with `http`) in the wild?  In the server Git history, it
looks like server v1.6.0 could send `sender_avatar_url` as a
relative URL if it's a local upload, or if there's no upload and
ENABLE_GRAVATAR is false.  But commit 1.7.0~3388, released Oct
2017, fixed that; since then it's always absolute.
  • Loading branch information
gnprice committed Mar 7, 2019
1 parent 58e074b commit cee71e0
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ private static Notification.Builder getNotificationBuilder(
String time = fcmMessage.getTime();
String stream = fcmMessage.getStream();
String topic = fcmMessage.getTopic();
String baseURL = fcmMessage.getBaseURL();
int totalMessagesCount = extractTotalMessagesCount(conversations);

if (BuildConfig.DEBUG) {
Expand All @@ -173,7 +172,7 @@ private static Notification.Builder getNotificationBuilder(
}
if (avatarURL.startsWith("http")) {
Bitmap avatar = fetchAvatar(NotificationHelper.sizedURL(context,
avatarURL, 64, baseURL));
avatarURL, 64));
if (avatar != null) {
builder.setLargeIcon(avatar);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.zulipmobile.notifications

import android.os.Bundle
import java.net.MalformedURLException
import java.net.URL


/**
Expand Down Expand Up @@ -43,9 +45,6 @@ internal class MessageFcmMessage private constructor(
val event: String?
get() = bundle.getString("event")

val baseURL: String?
get() = bundle.getString("base_url")

protected fun copy(): MessageFcmMessage {
return fromBundle(bundle)
}
Expand All @@ -64,10 +63,17 @@ internal class MessageFcmMessage private constructor(
else -> throw FcmMessageParseException("unexpected recipient_type: $recipientType")
}

val avatarURL = bundle.requireString("sender_avatar_url")
try {
URL(avatarURL)
} catch (e: MalformedURLException) {
throw FcmMessageParseException("invalid sender_avatar_url: $avatarURL")
}

return MessageFcmMessage(
email = bundle.requireString("sender_email"),
senderFullName = bundle.requireString("sender_full_name"),
avatarURL = bundle.requireString("sender_avatar_url"),
avatarURL = avatarURL,

recipientType = recipientType,
isGroupMessage = recipientType == "private" && bundle.getString("pm_users") != null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,30 +45,19 @@ static Bitmap fetch(URL url) throws IOException {
return null;
}

static URL sizedURL(Context context, String url, float dpSize, String baseUrl) {
static URL sizedURL(Context context, String url, float dpSize) {
// From http://stackoverflow.com/questions/4605527/
Resources r = context.getResources();
float px = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP,
dpSize, r.getDisplayMetrics());
try {
return new URL(addHost(url, baseUrl) + "&s=" + px);
return new URL(url + "&s=" + px);
} catch (MalformedURLException e) {
Log.e(TAG, "ERROR: " + e.toString());
return null;
}
}

private static String addHost(String url, String baseURL) {
if (!url.startsWith("http")) {
if (baseURL.endsWith("/")) {
url = baseURL.substring(0, baseURL.length() - 1) + url;
} else {
url = baseURL + url;
}
}
return url;
}


private static String extractName(String key) {
return key.split(":")[0];
Expand Down

0 comments on commit cee71e0

Please sign in to comment.