Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Nullsafe FIXMEs for MultipartStreamReader.java #50056

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@

package com.facebook.react.devsupport;

import com.facebook.infer.annotation.Nullsafe;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import okio.Buffer;
import okio.BufferedSource;
import okio.ByteString;

/** Utility class to parse the body of a response of type multipart/mixed. */
@Nullsafe(Nullsafe.Mode.LOCAL)
class MultipartStreamReader {
// Standard line separator for HTTP.
private static final String CRLF = "\r\n";
Expand Down Expand Up @@ -60,7 +63,7 @@ private void emitChunk(Buffer chunk, boolean done, ChunkListener listener) throw
ByteString marker = ByteString.encodeUtf8(CRLF + CRLF);
long indexOfMarker = chunk.indexOf(marker);
if (indexOfMarker == -1) {
listener.onChunkComplete(null, chunk, done);
listener.onChunkComplete(Collections.emptyMap(), chunk, done);
} else {
Buffer headers = new Buffer();
Buffer body = new Buffer();
Expand Down Expand Up @@ -148,6 +151,7 @@ public boolean readAllParts(ChunkListener listener) throws IOException {
Buffer chunk = new Buffer();
content.skip(chunkStart);
content.read(chunk, length);
// NULLSAFE_FIXME[Parameter Not Nullable]
emitProgress(currentHeaders, chunk.size() - currentHeadersLength, true, listener);
emitChunk(chunk, isCloseDelimiter, listener);
currentHeaders = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
package com.facebook.react.devsupport;

import androidx.annotation.Nullable;
import com.facebook.infer.annotation.Assertions;
import com.facebook.infer.annotation.Nullsafe;
import com.facebook.react.bridge.JavaOnlyArray;
import com.facebook.react.bridge.JavaOnlyMap;
import com.facebook.react.bridge.ReadableArray;
Expand All @@ -26,6 +28,7 @@
import org.json.JSONObject;

/** Helper class converting JS and Java stack traces into arrays of {@link StackFrame} objects. */
@Nullsafe(Nullsafe.Mode.LOCAL)
public class StackTraceHelper {

public static final String COLUMN_KEY = "column";
Expand All @@ -49,27 +52,29 @@ public class StackTraceHelper {

/** Represents a generic entry in a stack trace, be it originally from JS or Java. */
public static class StackFrameImpl implements StackFrame {
private final String mFile;
@Nullable private final String mFile;
private final String mMethod;
private final int mLine;
private final int mColumn;
private final String mFileName;
@Nullable private final String mFileName;
private final boolean mIsCollapsed;

private StackFrameImpl(String file, String method, int line, int column, boolean isCollapsed) {
private StackFrameImpl(
@Nullable String file, String method, int line, int column, boolean isCollapsed) {
mFile = file;
mMethod = method;
mLine = line;
mColumn = column;
mFileName = file != null ? new File(file).getName() : "";
mFileName = file != null ? new File(file).getName() : null;
mIsCollapsed = isCollapsed;
}

private StackFrameImpl(String file, String method, int line, int column) {
private StackFrameImpl(@Nullable String file, String method, int line, int column) {
this(file, method, line, column, false);
}

private StackFrameImpl(String file, String fileName, String method, int line, int column) {
private StackFrameImpl(
@Nullable String file, @Nullable String fileName, String method, int line, int column) {
mFile = file;
mFileName = fileName;
mMethod = method;
Expand All @@ -84,7 +89,8 @@ private StackFrameImpl(String file, String fileName, String method, int line, in
* <p>JS traces return the full path to the file here, while Java traces only return the file
* name (the path is not known).
*/
public String getFile() {
@Override
public @Nullable String getFile() {
return mFile;
}

Expand All @@ -109,7 +115,8 @@ public int getColumn() {
* <p>For JS traces this is different from {@link #getFile()} in that it only returns the file
* name, not the full path. For Java traces there is no difference.
*/
public String getFileName() {
@Override
public @Nullable String getFileName() {
return mFileName;
}

Expand All @@ -119,9 +126,10 @@ public boolean isCollapsed() {

/** Convert the stack frame to a JSON representation. */
public JSONObject toJSON() {
String file = getFile();
return new JSONObject(
MapBuilder.of(
"file", getFile(),
"file", (file == null) ? "" : file,
"methodName", getMethod(),
"lineNumber", getLine(),
"column", getColumn(),
Expand All @@ -136,25 +144,33 @@ public JSONObject toJSON() {
public static StackFrame[] convertJsStackTrace(@Nullable ReadableArray stack) {
int size = stack != null ? stack.size() : 0;
StackFrame[] result = new StackFrame[size];
for (int i = 0; i < size; i++) {
ReadableType type = stack.getType(i);
if (type == ReadableType.Map) {
ReadableMap frame = stack.getMap(i);
String methodName = frame.getString("methodName");
String fileName = frame.getString("file");
boolean collapse =
frame.hasKey("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse");
int lineNumber = -1;
if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) {
lineNumber = frame.getInt(LINE_NUMBER_KEY);
}
int columnNumber = -1;
if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) {
columnNumber = frame.getInt(COLUMN_KEY);
if (stack != null) {
for (int i = 0; i < size; i++) {
ReadableType type = stack.getType(i);
if (type == ReadableType.Map) {
ReadableMap frame = stack.getMap(i);
Assertions.assertNotNull(frame);
String methodName = frame.getString("methodName");
String fileName = frame.getString("file");
Assertions.assertNotNull(fileName);
Assertions.assertNotNull(methodName);
boolean collapse =
frame.hasKey("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse");
int lineNumber = -1;
if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) {
lineNumber = frame.getInt(LINE_NUMBER_KEY);
}
int columnNumber = -1;
if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) {
columnNumber = frame.getInt(COLUMN_KEY);
}
result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse);
} else if (type == ReadableType.String) {
String stackFrame = stack.getString(i);
if (stackFrame != null) {
result[i] = new StackFrameImpl(null, stackFrame, -1, -1);
}
}
result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse);
} else if (type == ReadableType.String) {
result[i] = new StackFrameImpl(null, stack.getString(i), -1, -1);
}
}
return result;
Expand Down Expand Up @@ -203,15 +219,22 @@ public static StackFrame[] convertJsStackTrace(String stack) {
} else if (matcher1.find()) {
matcher = matcher1;
} else {
result[i] = new StackFrameImpl(null, stackTrace[i], -1, -1);
String unmatchedStackFrame = stackTrace[i];
if (unmatchedStackFrame != null) {
result[i] = new StackFrameImpl(null, unmatchedStackFrame, -1, -1);
}
continue;
}
String fileName = matcher.group(2);
String methodName = matcher.group(1) == null ? "(unknown)" : matcher.group(1);
String lineString = matcher.group(3);
String columnString = matcher.group(4);
if (methodName == null || fileName == null || lineString == null || columnString == null) {
continue;
}
result[i] =
new StackFrameImpl(
matcher.group(2),
matcher.group(1) == null ? "(unknown)" : matcher.group(1),
Integer.parseInt(matcher.group(3)),
Integer.parseInt(matcher.group(4)));
fileName, methodName, Integer.parseInt(lineString), Integer.parseInt(columnString));
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public interface StackFrame {
public val file: String?

/** Get the name of the method this frame points to. */
public val method: String?
public val method: String

/** Get the line number this frame points to in the file returned by [.getFile]. */
public val line: Int
Expand All @@ -40,5 +40,5 @@ public interface StackFrame {
public val isCollapsed: Boolean

/** Convert the stack frame to a JSON representation. */
public fun toJSON(): JSONObject?
public fun toJSON(): JSONObject
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class MultipartStreamReaderTest {

val callback: CallCountTrackingChunkCallback =
object : CallCountTrackingChunkCallback() {
override fun onChunkComplete(headers: Map<String, String>?, body: Buffer, done: Boolean) {
override fun onChunkComplete(headers: Map<String, String>, body: Buffer, done: Boolean) {
super.onChunkComplete(headers, body, done)

assertThat(done).isTrue
Expand Down Expand Up @@ -68,7 +68,7 @@ class MultipartStreamReaderTest {

val callback: CallCountTrackingChunkCallback =
object : CallCountTrackingChunkCallback() {
override fun onChunkComplete(headers: Map<String, String>?, body: Buffer, done: Boolean) {
override fun onChunkComplete(headers: Map<String, String>, body: Buffer, done: Boolean) {
super.onChunkComplete(headers, body, done)

assertThat(done).isEqualTo(callCount == 3)
Expand Down Expand Up @@ -125,7 +125,7 @@ class MultipartStreamReaderTest {
var callCount = 0
private set

override fun onChunkComplete(headers: Map<String, String>?, body: Buffer, done: Boolean) {
override fun onChunkComplete(headers: Map<String, String>, body: Buffer, done: Boolean) {
callCount++
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class StackTraceHelperTest {
fun testParseStackFrameWithInvalidFrame() {
val frame = StackTraceHelper.convertJsStackTrace("Test.bundle:ten:twenty").get(0)
assertThat(frame.method).isEqualTo("Test.bundle:ten:twenty")
assertThat(frame.fileName).isEqualTo("")
assertThat(frame.fileName).isEqualTo(null)
assertThat(frame.line).isEqualTo(-1)
assertThat(frame.column).isEqualTo(-1)
}
Expand All @@ -56,7 +56,7 @@ class StackTraceHelperTest {
fun testParseStackFrameWithNativeCodeFrame() {
val frame = StackTraceHelper.convertJsStackTrace("forEach@[native code]").get(0)
assertThat(frame.method).isEqualTo("forEach@[native code]")
assertThat(frame.fileName).isEqualTo("")
assertThat(frame.fileName).isEqualTo(null)
assertThat(frame.line).isEqualTo(-1)
assertThat(frame.column).isEqualTo(-1)
}
Expand Down
Loading