Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Added exception if you try to reply with a non-direct ByteBuffer. #25952

Merged
gaaclarke merged 4 commits into from
May 12, 2021
Merged

Added exception if you try to reply with a non-direct ByteBuffer. #25952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9aed9ab
Added exception if you try to reply with a non-direct ByteBuffer.
gaaclarke May 5, 2021
2981e4d
added dcheck
gaaclarke May 5, 2021
49c597d
added a test
gaaclarke May 5, 2021
28b214a
format
gaaclarke May 5, 2021
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
5 changes: 4 additions & 1 deletion shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,8 +902,11 @@ private native void nativeInvokePlatformMessageEmptyResponseCallback(
// TODO(mattcarroll): differentiate between channel responses and platform responses.
@UiThread
public void invokePlatformMessageResponseCallback(
int responseId, @Nullable ByteBuffer message, int position) {
int responseId, @NonNull ByteBuffer message, int position) {
ensureRunningOnMainThread();
if (!message.isDirect()) {
Copy link

@blasten blasten May 5, 2021

Choose a reason for hiding this comment

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

does it also fix flutter/flutter#81875?

Copy link
Member Author

@gaaclarke gaaclarke May 5, 2021

Choose a reason for hiding this comment

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

Half of the problem, there were 2. The position problem and the direct buffer problem.

Copy link

@blasten blasten May 5, 2021

Choose a reason for hiding this comment

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

do we want to remove the ability to set null?

Copy link
Member Author

@gaaclarke gaaclarke May 5, 2021

Choose a reason for hiding this comment

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

Yep, check out where this function is actually used: https://github.com/gaaclarke/engine/blob/7219fc0932a238cee073bb20f19ccd8f8729d022/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java#L160:L161 There is a separate function you are supposed to use if you want to send null.

Copy link

@blasten blasten May 5, 2021

Choose a reason for hiding this comment

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

sg

throw new IllegalArgumentException("Expected a direct ByteBuffer.");
}
if (isAttached()) {
nativeInvokePlatformMessageResponseCallback(
nativeShellHolderId, responseId, message, position);
Expand Down
1 change: 1 addition & 0 deletions shell/platform/android/platform_view_android.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ void PlatformViewAndroid::InvokePlatformMessageResponseCallback(
return;
uint8_t* response_data =
static_cast<uint8_t*>(env->GetDirectBufferAddress(java_response_data));
FML_DCHECK(response_data != nullptr);
std::vector<uint8_t> response = std::vector<uint8_t>(
response_data, response_data + java_response_position);
auto message_response = std::move(it->second);
Expand Down
8 changes: 8 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.flutter.embedding.engine.systemchannels.LocalizationChannel;
import io.flutter.plugin.localization.LocalizationPlugin;
import io.flutter.plugin.platform.PlatformViewsController;
import java.nio.ByteBuffer;
import java.util.Locale;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Test;
Expand Down Expand Up @@ -237,4 +238,11 @@ public void createOverlaySurface__callsPlatformViewsController() {
// --- Verify Results ---
verify(platformViewsController, times(1)).createOverlaySurface();
}

@Test(expected = IllegalArgumentException.class)
public void invokePlatformMessageResponseCallback__wantsDirectBuffer() {
FlutterJNI flutterJNI = new FlutterJNI();
ByteBuffer buffer = ByteBuffer.allocate(4);
flutterJNI.invokePlatformMessageResponseCallback(0, buffer, buffer.position());
}
}