From d95baa3ee95121b227497d0fe63a0481b7b8b0e8 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:46:38 -0700 Subject: [PATCH] CronetDataSource: Fix thread safety issue with requestProperties The access in fillCurrentRequestPostBody wasn't protected with synchronization. Furthermore, just synchronizing it wouldn't be sufficient, since what we really need to check is whether the Content-Type header has been added to the UrlRequest.Builder. The contents of requestProperties may have changed between the headers being added to UrlRequest.Builder and the call to fillCurrentRequestPostBody. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135376904 --- .../ext/cronet/CronetDataSource.java | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index b563b70419..594ea8cb17 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -212,13 +212,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou operation.close(); resetConnectTimeout(); - currentDataSpec = dataSpec; - UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(dataSpec.uri.toString(), this, - executor, cronetEngine); - fillCurrentRequestHeader(urlRequestBuilder); - fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); - currentUrlRequest = urlRequestBuilder.build(); + currentUrlRequest = buildRequest(dataSpec); currentUrlRequest.start(); boolean requestStarted = blockUntilConnectTimeout(); @@ -423,36 +418,35 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // Internal methods. - private void fillCurrentRequestHeader(UrlRequest.Builder urlRequestBuilder) { + private UrlRequest buildRequest(DataSpec dataSpec) throws OpenException { + UrlRequest.Builder requestBuilder = new UrlRequest.Builder(dataSpec.uri.toString(), this, + executor, cronetEngine); + // Set the headers. synchronized (requestProperties) { + if (dataSpec.postBody != null && !requestProperties.containsKey(CONTENT_TYPE)) { + throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); + } for (Entry headerEntry : requestProperties.entrySet()) { - urlRequestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); + requestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); } } - if (currentDataSpec.position == 0 && currentDataSpec.length == C.LENGTH_UNSET) { - // Not required. - return; + // Set the Range header. + if (currentDataSpec.position != 0 || currentDataSpec.length != C.LENGTH_UNSET) { + StringBuilder rangeValue = new StringBuilder(); + rangeValue.append("bytes="); + rangeValue.append(currentDataSpec.position); + rangeValue.append("-"); + if (currentDataSpec.length != C.LENGTH_UNSET) { + rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); + } + requestBuilder.addHeader("Range", rangeValue.toString()); } - StringBuilder rangeValue = new StringBuilder(); - rangeValue.append("bytes="); - rangeValue.append(currentDataSpec.position); - rangeValue.append("-"); - if (currentDataSpec.length != C.LENGTH_UNSET) { - rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); + // Set the body. + if (dataSpec.postBody != null) { + requestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), + executor); } - urlRequestBuilder.addHeader("Range", rangeValue.toString()); - } - - private void fillCurrentRequestPostBody(UrlRequest.Builder urlRequestBuilder, DataSpec dataSpec) - throws HttpDataSourceException { - if (dataSpec.postBody == null) { - return; - } - if (!requestProperties.containsKey(CONTENT_TYPE)) { - throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); - } - urlRequestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), - executor); + return requestBuilder.build(); } private boolean blockUntilConnectTimeout() {