diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 3a0742a26d..cae954db13 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -202,7 +202,7 @@ envoy_stream_intel PlatformBridgeFilter::streamIntel() { RELEASE_ASSERT(decoder_callbacks_, "StreamInfo accessed before filter callbacks are set"); auto& info = decoder_callbacks_->streamInfo(); // FIXME: Stream handle cannot currently be set from the filter context. - envoy_stream_intel stream_intel{-1, -1, 0}; + envoy_stream_intel stream_intel{-1, -1, 0, 0}; if (info.upstreamInfo()) { stream_intel.connection_id = info.upstreamInfo()->upstreamConnectionId().value_or(-1); } diff --git a/library/common/http/client.cc b/library/common/http/client.cc index 0095bd57c9..d18262cf82 100644 --- a/library/common/http/client.cc +++ b/library/common/http/client.cc @@ -41,23 +41,27 @@ void Client::DirectStreamCallbacks::encodeHeaders(const ResponseHeaderMap& heade ASSERT(http_client_.getStream(direct_stream_.stream_handle_, GetStreamFilters::ALLOW_FOR_ALL_STREAMS)); - direct_stream_.saveLatestStreamIntel(); - if (end_stream) { - closeStream(); - } + // Capture some metadata before potentially closing the stream. absl::string_view alpn = ""; - uint64_t response_status = Utility::getResponseStatus(headers); - if (direct_stream_.request_decoder_ && - direct_stream_.request_decoder_->streamInfo().upstreamInfo() && - direct_stream_.request_decoder_->streamInfo().upstreamInfo()->upstreamSslConnection()) { - alpn = direct_stream_.request_decoder_->streamInfo() - .upstreamInfo() - ->upstreamSslConnection() - ->alpn(); + if (direct_stream_.request_decoder_) { + direct_stream_.saveLatestStreamIntel(); + const auto& info = direct_stream_.request_decoder_->streamInfo(); + // Set the initial number of bytes consumed for the non terminal callbacks. + direct_stream_.stream_intel_.consumed_bytes_from_response = + info.getUpstreamBytesMeter() ? info.getUpstreamBytesMeter()->headerBytesReceived() : 0; + // Capture the alpn if available. + if (info.upstreamInfo() && info.upstreamInfo()->upstreamSslConnection()) { + alpn = info.upstreamInfo()->upstreamSslConnection()->alpn(); + } + } + + if (end_stream) { + closeStream(); } // Track success for later bookkeeping (stream could still be reset). + uint64_t response_status = Utility::getResponseStatus(headers); success_ = CodeUtility::is2xx(response_status); ENVOY_LOG(debug, "[S{}] dispatching to platform response headers for stream (end_stream={}):\n{}", @@ -123,6 +127,8 @@ void Client::DirectStreamCallbacks::sendDataToBridge(Buffer::Instance& data, boo // Cap by bytes_to_send_ if and only if applying explicit flow control. uint32_t bytes_to_send = calculateBytesToSend(data, bytes_to_send_); + // Update the number of bytes consumed by this non terminal callback. + direct_stream_.stream_intel_.consumed_bytes_from_response += bytes_to_send; // Only send end stream if all data is being sent. bool send_end_stream = end_stream && (bytes_to_send == data.length()); diff --git a/library/common/http/client.h b/library/common/http/client.h index 0246333a10..e66c065628 100644 --- a/library/common/http/client.h +++ b/library/common/http/client.h @@ -279,7 +279,7 @@ class Client : public Logger::Loggable { // read faster than the mobile caller can process it. bool explicit_flow_control_ = false; // Latest intel data retrieved from the StreamInfo. - envoy_stream_intel stream_intel_{-1, -1, 0}; + envoy_stream_intel stream_intel_{-1, -1, 0, 0}; envoy_final_stream_intel envoy_final_stream_intel_{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, 0}; StreamInfo::BytesMeterSharedPtr bytes_meter_; diff --git a/library/common/jni/jni_utility.cc b/library/common/jni/jni_utility.cc index 1cd4eaba0c..712269fb18 100644 --- a/library/common/jni/jni_utility.cc +++ b/library/common/jni/jni_utility.cc @@ -85,12 +85,13 @@ jbyteArray native_data_to_array(JNIEnv* env, envoy_data data) { } jlongArray native_stream_intel_to_array(JNIEnv* env, envoy_stream_intel stream_intel) { - jlongArray j_array = env->NewLongArray(3); + jlongArray j_array = env->NewLongArray(4); jlong* critical_array = static_cast(env->GetPrimitiveArrayCritical(j_array, nullptr)); RELEASE_ASSERT(critical_array != nullptr, "unable to allocate memory in jni_utility"); critical_array[0] = static_cast(stream_intel.stream_id); critical_array[1] = static_cast(stream_intel.connection_id); critical_array[2] = static_cast(stream_intel.attempt_count); + critical_array[3] = static_cast(stream_intel.consumed_bytes_from_response); // Here '0' (for which there is no named constant) indicates we want to commit the changes back // to the JVM and free the c array, where applicable. env->ReleasePrimitiveArrayCritical(j_array, critical_array, 0); diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index d664795885..11b0d9e735 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -153,6 +153,12 @@ typedef struct { int64_t connection_id; // The number of internal attempts to carry out a request/operation. 0 if not present. uint64_t attempt_count; + // Number of bytes consumed by the non terminal callbacks out of the response. + // NOTE: on terminal callbacks (on_complete, on_error_, on_cancel), this value will not be equal + // to envoy_final_stream_intel.received_byte_count. The latter represents the real number + // of bytes received before decompression. consumed_bytes_from_response omits the number + // number of bytes related to the Status Line, and is after decompression. + uint64_t consumed_bytes_from_response; } envoy_stream_intel; /** diff --git a/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java b/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java index 3e6f35a5d8..d96d311bc2 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java +++ b/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java @@ -6,11 +6,13 @@ class EnvoyStreamIntelImpl implements EnvoyStreamIntel { private long streamId; private long connectionId; private long attemptCount; + private long consumedBytesFromResponse; EnvoyStreamIntelImpl(long[] values) { streamId = values[0]; connectionId = values[1]; attemptCount = values[2]; + consumedBytesFromResponse = values[3]; } @Override @@ -27,4 +29,9 @@ public long getConnectionId() { public long getAttemptCount() { return attemptCount; } + + @Override + public long getConsumedBytesFromResponse() { + return consumedBytesFromResponse; + } } diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java index 15c9f8795c..50c3f621bf 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java @@ -2,6 +2,8 @@ /** * Exposes internal HTTP stream metrics, context, and other details sent once on stream end. + * + * Note: a value of -1 means "not present" for any field where the name is suffixed with "Ms". */ public interface EnvoyFinalStreamIntel { /* diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java index 5a8a305350..71f119bf71 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java @@ -4,6 +4,7 @@ * Exposes internal HTTP stream metrics, context, and other details. */ public interface EnvoyStreamIntel { + /** * An internal identifier for the stream. */ @@ -18,4 +19,14 @@ public interface EnvoyStreamIntel { * The number of internal attempts to carry out a request/operation. */ public long getAttemptCount(); + + /** + * The number of bytes consumed by the non terminal callbacks, from the response. + * + *

>NOTE: on terminal callbacks (on_complete, on_error_, on_cancel), this value will not be + * equal to {@link EnvoyFinalStreamIntel#getReceivedByteCount()}. The latter represents the real + * number of bytes received before decompression. getConsumedBytesFromResponse() omits the number + * number of bytes related to the Status Line, and is after decompression. + */ + public long getConsumedBytesFromResponse(); } diff --git a/library/java/org/chromium/net/impl/CronetUrlRequest.java b/library/java/org/chromium/net/impl/CronetUrlRequest.java index 4190214389..787e51c857 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequest.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequest.java @@ -16,6 +16,7 @@ import java.util.AbstractMap; import java.util.AbstractMap.SimpleEntry; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -29,6 +30,7 @@ import org.chromium.net.CallbackException; import org.chromium.net.CronetException; import org.chromium.net.InlineExecutionProhibitedException; +import org.chromium.net.RequestFinishedInfo; import org.chromium.net.UploadDataProvider; /** UrlRequest, backed by Envoy-Mobile. */ @@ -88,7 +90,7 @@ public final class CronetUrlRequest extends UrlRequestBase { private final String mUserAgent; private final HeadersList mRequestHeaders = new HeadersList(); - private final List mUrlChain = new ArrayList<>(); + private final Collection mRequestAnnotations; private final CronetUrlRequestContext mRequestContext; private final AtomicBoolean mWaitingOnRedirect = new AtomicBoolean(false); private final AtomicBoolean mWaitingOnRead = new AtomicBoolean(false); @@ -111,11 +113,16 @@ public final class CronetUrlRequest extends UrlRequestBase { /* These don't change with redirects */ private String mInitialMethod; - private CronetUploadDataStream mUploadDataStream; private final Executor mUserExecutor; private final VersionSafeCallbacks.UrlRequestCallback mCallback; + private final String mInitialUrl; + private final VersionSafeCallbacks.RequestFinishedInfoListener mRequestFinishedListener; private final ConditionVariable mStartBlock = new ConditionVariable(); + private CronetUploadDataStream mUploadDataStream; + + private volatile CronetException mException; + /** * Holds a subset of StatusValues - {@link State#STARTED} can represent {@link * Status#SENDING_REQUEST} or {@link Status#WAITING_FOR_RESPONSE}. While the distinction isn't @@ -127,13 +134,16 @@ public final class CronetUrlRequest extends UrlRequestBase { * only used with the STARTED state, so it's inconsequential. */ @StatusValues private volatile int mAdditionalStatusDetails = Status.INVALID; - private final AtomicReference mError = new AtomicReference<>(); /* These change with redirects. */ private final AtomicReference mStream = new AtomicReference<>(); + private final List mUrlChain = new ArrayList<>(); + private EnvoyFinalStreamIntel mEnvoyFinalStreamIntel; + private long mBytesReceivedFromRedirects = 0; + private long mBytesReceivedFromLastRedirect = 0; private CronvoyHttpCallbacks mCronvoyCallbacks; private String mCurrentUrl; - private UrlResponseInfoImpl mUrlResponseInfo; + private volatile UrlResponseInfoImpl mUrlResponseInfo; private String mPendingRedirectUrl; /** @@ -142,8 +152,9 @@ public final class CronetUrlRequest extends UrlRequestBase { */ CronetUrlRequest(CronetUrlRequestContext cronvoyEngine, Callback callback, Executor executor, String url, String userAgent, boolean allowDirectExecutor, - boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet, - int trafficStatsUid) { + Collection connectionAnnotations, boolean trafficStatsTagSet, + int trafficStatsTag, boolean trafficStatsUidSet, int trafficStatsUid, + RequestFinishedInfo.Listener requestFinishedListener) { if (url == null) { throw new NullPointerException("URL is required"); } @@ -154,11 +165,17 @@ public final class CronetUrlRequest extends UrlRequestBase { throw new NullPointerException("Executor is required"); } mCallback = new VersionSafeCallbacks.UrlRequestCallback(callback); + mRequestFinishedListener = + requestFinishedListener != null + ? new VersionSafeCallbacks.RequestFinishedInfoListener(requestFinishedListener) + : null; mRequestContext = cronvoyEngine; mAllowDirectExecutor = allowDirectExecutor; mUserExecutor = executor; + mInitialUrl = url; mCurrentUrl = url; mUserAgent = userAgent; + mRequestAnnotations = connectionAnnotations; } @Override @@ -305,8 +322,8 @@ public boolean isDone() { @Override public void getStatus(StatusListener listener) { + @StatusValues int extraStatus = mAdditionalStatusDetails; @State int state = mState.get(); - int extraStatus = mAdditionalStatusDetails; @StatusValues final int status; switch (state) { @@ -401,7 +418,6 @@ private static int determineNextErrorState(boolean streamEnded, @State int origi } private void enterErrorState(CronetException error) { - mError.compareAndSet(null, error); @State int originalState; @State int updatedState; do { @@ -414,6 +430,7 @@ private void enterErrorState(CronetException error) { if (isTerminalState(originalState)) { return; } + mException = error; fireCloseUploadDataProvider(); if (updatedState == State.ERROR_PENDING_CANCEL) { CronvoyHttpCallbacks cronvoyCallbacks = this.mCronvoyCallbacks; @@ -469,12 +486,17 @@ private void fireCloseUploadDataProvider() { } } + // This method is only called when in STARTED state. This means a "cancel" request won't be + // executed immediately - that quite important here, otherwise this would lead to unfortunate + // race conditions. A "cancel" request will then be honnored on the first callback. private void fireOpenConnection() { if (mInitialMethod == null) { mInitialMethod = "GET"; } + mUrlResponseInfo = null; + mEnvoyFinalStreamIntel = null; + mBytesReceivedFromRedirects += mBytesReceivedFromLastRedirect; mAdditionalStatusDetails = Status.CONNECTING; - mUrlResponseInfo = new UrlResponseInfoImpl(); mUrlChain.add(mCurrentUrl); Map> envoyRequestHeaders = buildEnvoyRequestHeaders(mInitialMethod, mRequestHeaders, mUploadDataStream, mUserAgent, @@ -568,7 +590,7 @@ public void run() { try { mCallback.onCanceled(CronetUrlRequest.this, mUrlResponseInfo); } catch (Exception exception) { - Log.e(TAG, "Exception in onCanceled method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onCanceled method", exception); } } }; @@ -582,7 +604,7 @@ public void run() { try { mCallback.onSucceeded(CronetUrlRequest.this, mUrlResponseInfo); } catch (Exception exception) { - Log.e(TAG, "Exception in onSucceeded method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onSucceeded method", exception); } } }; @@ -594,9 +616,9 @@ void onFailed() { @Override public void run() { try { - mCallback.onFailed(CronetUrlRequest.this, mUrlResponseInfo, mError.get()); + mCallback.onFailed(CronetUrlRequest.this, mUrlResponseInfo, mException); } catch (Exception exception) { - Log.e(TAG, "Exception in onFailed method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onFailed method", exception); } } }; @@ -631,6 +653,19 @@ private boolean streamEnded() { return cronvoyCallbacks != null && cronvoyCallbacks.mEndStream; } + private void recordEnvoyFinalStreamIntel(EnvoyFinalStreamIntel envoyFinalStreamIntel) { + mEnvoyFinalStreamIntel = envoyFinalStreamIntel; + if (mUrlResponseInfo != null) { // Null if cancelled before receiving a Response. + mUrlResponseInfo.setReceivedByteCount(envoyFinalStreamIntel.getReceivedByteCount() + + mBytesReceivedFromRedirects); + } + } + + private void recordEnvoyStreamIntel(EnvoyStreamIntel envoyStreamIntel) { + mUrlResponseInfo.setReceivedByteCount(envoyStreamIntel.getConsumedBytesFromResponse() + + mBytesReceivedFromRedirects); + } + private static class HeadersList extends ArrayList> {} private static class DirectExecutor implements Executor { @@ -666,9 +701,8 @@ public Executor getExecutor() { @Override public void onHeaders(Map> headers, boolean endStream, EnvoyStreamIntel streamIntel) { - if (isAbandoned()) { - return; - } + mUrlResponseInfo = new UrlResponseInfoImpl(); + recordEnvoyStreamIntel(streamIntel); mEndStream = endStream; List statuses = headers.get(":status"); final int responseCode = @@ -698,6 +732,7 @@ public void onHeaders(Map> headers, boolean endStream, } if (locationField != null) { + mBytesReceivedFromLastRedirect = streamIntel.getConsumedBytesFromResponse(); cancel(); // Abort the the original request - we are being redirected. } @@ -734,6 +769,7 @@ public void onData(ByteBuffer data, boolean endStream, EnvoyStreamIntel streamIn if (isAbandoned()) { return; } + recordEnvoyStreamIntel(streamIntel); mEndStream = endStream; @State int originalState; @State int updatedState; @@ -789,6 +825,7 @@ public void onError(int errorCode, String message, int attemptCount, if (isAbandoned()) { return; } + recordEnvoyFinalStreamIntel(finalStreamIntel); mEndStream = true; @State int originalState; @State int updatedState; @@ -811,6 +848,7 @@ public void onCancel(EnvoyStreamIntel streamIntel, EnvoyFinalStreamIntel finalSt if (isAbandoned()) { return; } + recordEnvoyFinalStreamIntel(finalStreamIntel); mEndStream = true; @State int originalState; @State int updatedState; @@ -852,7 +890,7 @@ public void onComplete(EnvoyStreamIntel streamIntel, EnvoyFinalStreamIntel final if (isAbandoned()) { return; } - mUrlResponseInfo.setReceivedByteCount(finalStreamIntel.getSentByteCount()); + recordEnvoyFinalStreamIntel(finalStreamIntel); if (successReady(SucceededState.ON_COMPLETE_RECEIVED)) { onSucceeded(); } @@ -900,7 +938,7 @@ void readData(int size) { */ void cancel() { EnvoyHTTPStream stream = mStream.get(); - if (isAbandoned() || mEndStream) { + if (this != mCronvoyCallbacks || mEndStream) { return; } @CancelState int oldState = mCancelState.getAndSet(CancelState.CANCELLED); @@ -936,6 +974,7 @@ private void setUrlResponseInfo(Map> responseHeaders, int r // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1426) set receivedByteCount // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1622) support proxy // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1546) negotiated protocol + // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1578) http caching mUrlResponseInfo.setResponseValues( new ArrayList<>(mUrlChain), responseCode, HttpReason.getReason(responseCode), Collections.unmodifiableList(headerList), false, selectedTransport, ":0"); diff --git a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java index d12b41a955..00e9f3a226 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java @@ -126,8 +126,8 @@ void setTaskToExecuteWhenInitializationIsCompleted(Runnable runnable) { int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener, int idempotency) { return new CronetUrlRequest(this, callback, executor, url, mUserAgent, allowDirectExecutor, - trafficStatsTagSet, trafficStatsTag, trafficStatsUidSet, - trafficStatsUid); + connectionAnnotations, trafficStatsTagSet, trafficStatsTag, + trafficStatsUidSet, trafficStatsUid, requestFinishedListener); } @Override diff --git a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt index 67cf7990bb..d85673347f 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt @@ -15,6 +15,7 @@ class MockStream internal constructor(underlyingStream: MockEnvoyHTTPStream) : S override fun getStreamId(): Long { return 0 } override fun getConnectionId(): Long { return 0 } override fun getAttemptCount(): Long { return 0 } + override fun getConsumedBytesFromResponse(): Long { return 0 } } private val mockFinalStreamIntel = object : EnvoyFinalStreamIntel { diff --git a/test/common/integration/client_integration_test.cc b/test/common/integration/client_integration_test.cc index 6e104cd2bb..0425f915a9 100644 --- a/test/common/integration/client_integration_test.cc +++ b/test/common/integration/client_integration_test.cc @@ -36,6 +36,8 @@ typedef struct { uint32_t on_complete_calls; uint32_t on_error_calls; uint32_t on_cancel_calls; + uint64_t on_header_consumed_bytes_from_response; + uint64_t on_complete_received_byte_count; std::string status; ConditionalInitializer* terminal_callback; } callbacks_called; @@ -62,12 +64,13 @@ class ClientIntegrationTest : public BaseIntegrationTest, }); bridge_callbacks_.context = &cc_; - bridge_callbacks_.on_headers = [](envoy_headers c_headers, bool, envoy_stream_intel, + bridge_callbacks_.on_headers = [](envoy_headers c_headers, bool, envoy_stream_intel intel, void* context) -> void* { Http::ResponseHeaderMapPtr response_headers = toResponseHeaders(c_headers); callbacks_called* cc_ = static_cast(context); cc_->on_headers_calls++; cc_->status = response_headers->Status()->value().getStringView(); + cc_->on_header_consumed_bytes_from_response = intel.consumed_bytes_from_response; return nullptr; }; bridge_callbacks_.on_data = [](envoy_data c_data, bool, envoy_stream_intel, @@ -77,9 +80,10 @@ class ClientIntegrationTest : public BaseIntegrationTest, release_envoy_data(c_data); return nullptr; }; - bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel, + bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel final_intel, void* context) -> void* { callbacks_called* cc_ = static_cast(context); + cc_->on_complete_received_byte_count = final_intel.received_byte_count; cc_->on_complete_calls++; cc_->terminal_callback->setReady(); return nullptr; @@ -143,7 +147,7 @@ name: api_listener Http::ClientPtr http_client_{}; envoy_http_callbacks bridge_callbacks_; ConditionalInitializer terminal_callback_; - callbacks_called cc_ = {0, 0, 0, 0, 0, "", &terminal_callback_}; + callbacks_called cc_ = {0, 0, 0, 0, 0, 0, 0, "", &terminal_callback_}; }; INSTANTIATE_TEST_SUITE_P(IpVersions, ClientIntegrationTest, @@ -206,6 +210,8 @@ TEST_P(ClientIntegrationTest, Basic) { ASSERT_EQ(cc_.status, "200"); ASSERT_EQ(cc_.on_data_calls, 2); ASSERT_EQ(cc_.on_complete_calls, 1); + ASSERT_EQ(cc_.on_header_consumed_bytes_from_response, 27); + ASSERT_EQ(cc_.on_complete_received_byte_count, 67); // stream_success gets charged for 2xx status codes. test_server_->waitForCounterEq("http.client.stream_success", 1); diff --git a/test/java/org/chromium/net/CronetUrlRequestTest.java b/test/java/org/chromium/net/CronetUrlRequestTest.java index 61e9249813..414d19d96a 100644 --- a/test/java/org/chromium/net/CronetUrlRequestTest.java +++ b/test/java/org/chromium/net/CronetUrlRequestTest.java @@ -220,7 +220,7 @@ public void testRedirectAsync() throws Exception { "header-value"); UrlResponseInfo expected = createUrlResponseInfo( - new String[] {NativeTestServer.getRedirectURL()}, "Found", 302, 73, "Content-Length", "92", + new String[] {NativeTestServer.getRedirectURL()}, "Found", 302, 72, "Content-Length", "92", "Location", "/success.txt", "redirect-header", "header-value"); mTestRule.assertResponseEquals(expected, callback.mRedirectResponseInfoList.get(0)); @@ -266,9 +266,10 @@ public void testRedirectAsync() throws Exception { assertEquals(ResponseStep.ON_SUCCEEDED, callback.mResponseStep); assertEquals(NativeTestServer.SUCCESS_BODY, callback.mResponseAsString); + // Original bytesReceived: 258 UrlResponseInfo urlResponseInfo = createUrlResponseInfo( new String[] {NativeTestServer.getRedirectURL(), NativeTestServer.getSuccessURL()}, "OK", - 200, 258, "Content-Length", "20", "Content-Type", "text/plain", + 200, 284, "Content-Length", "20", "Content-Type", "text/plain", "Access-Control-Allow-Origin", "*", "header-name", "header-value", "multi-header-name", "header-value1", "multi-header-name", "header-value2"); @@ -666,17 +667,19 @@ public void testMockMultiRedirect() throws Exception { assertEquals(2, callback.mRedirectResponseInfoList.size()); // Check first redirect (multiredirect.html -> redirect.html) + // Original receivedBytes: 76 UrlResponseInfo firstExpectedResponseInfo = createUrlResponseInfo( - new String[] {NativeTestServer.getMultiRedirectURL()}, "Found", 302, 76, "Content-Length", + new String[] {NativeTestServer.getMultiRedirectURL()}, "Found", 302, 75, "Content-Length", "92", "Location", "/redirect.html", "redirect-header0", "header-value"); UrlResponseInfo firstRedirectResponseInfo = callback.mRedirectResponseInfoList.get(0); mTestRule.assertResponseEquals(firstExpectedResponseInfo, firstRedirectResponseInfo); // Check second redirect (redirect.html -> success.txt) + // Original receivedBytes: 334 UrlResponseInfo secondExpectedResponseInfo = createUrlResponseInfo( new String[] {NativeTestServer.getMultiRedirectURL(), NativeTestServer.getRedirectURL(), NativeTestServer.getSuccessURL()}, - "OK", 200, 334, "Content-Length", "20", "Content-Type", "text/plain", + "OK", 200, 359, "Content-Length", "20", "Content-Type", "text/plain", "Access-Control-Allow-Origin", "*", "header-name", "header-value", "multi-header-name", "header-value1", "multi-header-name", "header-value2"); @@ -693,7 +696,7 @@ public void testMockNotFound() throws Exception { TestUrlRequestCallback callback = startAndWaitForComplete(NativeTestServer.getNotFoundURL()); UrlResponseInfo expected = createUrlResponseInfo(new String[] {NativeTestServer.getNotFoundURL()}, "Not Found", 404, - 140, "Content-Length", "96"); + 142, "Content-Length", "96"); mTestRule.assertResponseEquals(expected, callback.mResponseInfo); assertTrue(callback.mHttpResponseDataLength != 0); assertEquals(0, callback.mRedirectCount); @@ -2137,26 +2140,26 @@ public void testLegacyOnFailedCallback() throws Exception { final ConditionVariable done = new ConditionVariable(); UrlRequest.Callback callback = new UrlRequest.Callback() { @Override - public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, - String newLocationUrl) { + public void onSucceeded(UrlRequest request, UrlResponseInfo info) { failedExpectation.set(true); fail(); } @Override - public void onResponseStarted(UrlRequest request, UrlResponseInfo info) { + public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, + String newLocationUrl) { failedExpectation.set(true); fail(); } @Override - public void onReadCompleted(UrlRequest request, UrlResponseInfo info, ByteBuffer byteBuffer) { + public void onResponseStarted(UrlRequest request, UrlResponseInfo info) { failedExpectation.set(true); fail(); } @Override - public void onSucceeded(UrlRequest request, UrlResponseInfo info) { + public void onReadCompleted(UrlRequest request, UrlResponseInfo info, ByteBuffer byteBuffer) { failedExpectation.set(true); fail(); } diff --git a/test/java/org/chromium/net/testing/CronetTestRule.java b/test/java/org/chromium/net/testing/CronetTestRule.java index c25e46a45d..b15eece987 100644 --- a/test/java/org/chromium/net/testing/CronetTestRule.java +++ b/test/java/org/chromium/net/testing/CronetTestRule.java @@ -68,7 +68,7 @@ public static class CronetTestFramework { private static ExperimentalCronetEngine createEngine(Context context) { ExperimentalCronetEngine.Builder builder = new ExperimentalCronetEngine.Builder(context); - ((CronetEngineBuilderImpl)builder.getBuilderDelegate()).setLogLevel("info"); + ((CronetEngineBuilderImpl)builder.getBuilderDelegate()).setLogLevel("warning"); return builder.enableQuic(true).build(); } @@ -253,8 +253,7 @@ public void assertResponseEquals(UrlResponseInfo expected, UrlResponseInfo actua assertEquals(expected.getUrl(), actual.getUrl()); // Transferred bytes and proxy server are not supported in pure java if (!testingJavaImpl()) { - // TODO("https://github.com/envoyproxy/envoy-mobile/issues/1426"): uncomment the assert - // assertEquals(expected.getReceivedByteCount(), actual.getReceivedByteCount()); + assertEquals(expected.getReceivedByteCount(), actual.getReceivedByteCount()); assertEquals(expected.getProxyServer(), actual.getProxyServer()); // This is a place where behavior intentionally differs between native and java assertEquals(expected.getNegotiatedProtocol(), actual.getNegotiatedProtocol());