Merge pull request #175 from Azure/master

jofriedm-msft · web-flow · commit f1f89e30af78 · 2017-06-14T09:55:10.000-07:00
Bringing master to dev_breaking
diff --git a/ChangeLog.txt b/ChangeLog.txt
@@ -1,3 +1,9 @@
+2017.06.13 Version 5.3.0
+ * Fixed a bug where the transactional MD5 check would fail when downloading a range of blob or file and the recovery action is performed on a subsection of the range.
+ * Fixed leaking connections for table requests.
+ * Fixed a bug where retries happened immediately when experiencing a network exception uploading data or getting the response.
+ * Fixed a bug where the response stream was not being closed on nonretryable exceptions.
+
 2017.05.23 Version 5.2.0
  * Fixed Exists() calls on Shares and Directories to now populate metadata. This was already being done for Files.
  * Changed blob constants to support up to 256 MB on put blob for block blobs. The default value for put blob threshold has also been updated to half of the maximum, or 128 MB currently.
diff --git a/README.md b/README.md
@@ -30,7 +30,7 @@ To get the binaries of this library as distributed by Microsoft, ready for use w
 <dependency>
 	<groupId>com.microsoft.azure</groupId>
 	<artifactId>azure-storage</artifactId>
-	<version>5.2.0</version>
+	<version>5.3.0</version>
 </dependency>
 ```
 
diff --git a/microsoft-azure-storage-samples/pom.xml b/microsoft-azure-storage-samples/pom.xml
@@ -26,7 +26,7 @@
 		<dependency>
 			<groupId>com.microsoft.azure</groupId>
 			<artifactId>azure-storage</artifactId>
-			<version>5.2.0</version>
+			<version>5.3.0</version>
 		</dependency>
 		<dependency>
 			<groupId>com.microsoft.azure</groupId>
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/Constants.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/Constants.java
@@ -661,7 +661,7 @@ public static class HeaderConstants {
         /**
          * Specifies the value to use for UserAgent header.
          */
-        public static final String USER_AGENT_VERSION = "5.2.0";
+        public static final String USER_AGENT_VERSION = "5.3.0";
 
         /**
          * The default type for content-type and accept
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/RequestResult.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/RequestResult.java
@@ -173,6 +173,13 @@ public String getStatusMessage() {
      * @return A <code>java.util.Date</code> object which contains the stop date.
      */
     public Date getStopDate() {
+        if (this.stopDate == null)
+        {
+            // stop date was not initialized, most likely due to a network exception
+            // if this is null, retries are immediate
+            this.stopDate = new Date();
+        }
+
         return this.stopDate;
     }
 
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/blob/BlobEncryptStream.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/blob/BlobEncryptStream.java
@@ -145,7 +145,7 @@ public void write(byte[] data, int offset, int length) throws IOException {
 
     @Override
     public void write(InputStream sourceStream, long writeLength) throws IOException, StorageException {
-        Utility.writeToOutputStream(sourceStream, this, writeLength, false, false, this.opContext, this.options, false);      
+        Utility.writeToOutputStream(sourceStream, this, writeLength, false, false, this.opContext, this.options, false);
     }
 
     @Override
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/blob/CloudBlob.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/blob/CloudBlob.java
@@ -1402,7 +1402,7 @@ public Integer postProcessResponse(HttpURLConnection connection, CloudBlob blob,
                     // writeToOutputStream will update the currentRequestByteCount on this request in case a retry
                     // is needed and download should resume from that point
                     final StreamMd5AndLength descriptor = Utility.writeToOutputStream(streamRef, outStream, -1, false,
-                            validateMD5, context, options, true, this);
+                            validateMD5, context, options, true, this, this.getCurrentDescriptor());
 
                     // length was already checked by the NetworkInputStream, now check Md5
                     if (validateMD5 && !this.getContentMD5().equals(descriptor.getMd5())) {
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/core/Canonicalizer.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/core/Canonicalizer.java
@@ -25,6 +25,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import com.microsoft.azure.storage.Constants;
 import com.microsoft.azure.storage.StorageException;
@@ -45,6 +47,8 @@ abstract class Canonicalizer {
      */
     private static final int ExpectedTableCanonicalizedStringLength = 200;
 
+    private static final Pattern CRLF  = Pattern.compile("\r\n", Pattern.LITERAL);
+
     /**
      * Add x-ms- prefixed headers in a fixed order.
      * 
@@ -85,7 +89,8 @@ private static void addCanonicalizedHeaders(final HttpURLConnection conn, final
                 }
 
                 // Unfolding is simply removal of CRLF.
-                final String unfoldedValue = value.replace("\r\n", Constants.EMPTY_STRING);
+                final String unfoldedValue = CRLF.matcher(value)
+                    .replaceAll(Matcher.quoteReplacement(Constants.EMPTY_STRING));
 
                 // Append it to the canonicalized element string.
                 canonicalizedElement.append(delimiter);
@@ -251,6 +256,11 @@ protected static String getCanonicalizedResource(final java.net.URL address, fin
         final StringBuilder canonicalizedResource = new StringBuilder(resourcepath.toString());
 
         // query parameters
+        if (address.getQuery() == null  || !address.getQuery().contains("=")) {
+            //no query params.
+            return canonicalizedResource.toString();
+        }
+
         final Map<String, String[]> queryVariables = PathUtility.parseQueryString(address.getQuery());
 
         final Map<String, String> lowercasedKeyNameValue = new HashMap<String, String>();
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/core/ExecutionEngine.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/core/ExecutionEngine.java
@@ -115,13 +115,10 @@ public static <CLIENT_TYPE, PARENT_TYPE, RESULT_TYPE> RESULT_TYPE executeWithRet
                 currResult.setStartDate(new Date());
 
                 Logger.info(opContext, LogConstants.GET_RESPONSE);
-                try {
-                    currResult.setStatusCode(request.getResponseCode());
-                    currResult.setStatusMessage(request.getResponseMessage());
-                }
-                finally {
-                    currResult.setStopDate(new Date());
-                }
+
+                currResult.setStatusCode(request.getResponseCode());
+                currResult.setStatusMessage(request.getResponseMessage());
+                currResult.setStopDate(new Date());
 
                 currResult.setServiceRequestID(BaseResponse.getRequestId(request));
                 currResult.setEtag(BaseResponse.getEtag(request));
@@ -168,6 +165,7 @@ public static <CLIENT_TYPE, PARENT_TYPE, RESULT_TYPE> RESULT_TYPE executeWithRet
                             }
                         }
                     }
+
                     Logger.info(opContext, LogConstants.COMPLETE);
 
                     return result;
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/core/StorageRequest.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/core/StorageRequest.java
@@ -99,6 +99,11 @@ public abstract class StorageRequest<C, P, R> {
      */
     private String contentMD5 = null;
 
+    /**
+     * Holds the descriptor which contains the stream length and MD5 hash.
+     */
+    private StreamMd5AndLength currentDescriptor = null;
+
     /**
      * Denotes the StorageUri of the request
      */
@@ -212,6 +217,11 @@ public final String getContentMD5() {
         return this.contentMD5;
     }
 
+    /**
+     * @return the current descriptor which contains the stream length and MD5 hash.
+     */
+    protected StreamMd5AndLength getCurrentDescriptor() { return this.currentDescriptor; }
+
     /**
      * @return the location mode used to decide which location the request should be sent to.
      */
@@ -457,6 +467,14 @@ public void setContentMD5(String contentMD5) {
         this.contentMD5 = contentMD5;
     }
 
+    /**
+     * @param currentDescriptor
+     *              the descriptor value
+     */
+    protected void setCurrentDescriptor(StreamMd5AndLength currentDescriptor) {
+        this.currentDescriptor = currentDescriptor;
+    }
+
     /**
      * @param etagLockCondition
      *            the locked ETag condition
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/core/Utility.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/core/Utility.java
@@ -70,6 +70,20 @@
  * RESERVED FOR INTERNAL USE. A class which provides utility methods.
  */
 public final class Utility {
+
+    /**
+     * Thread local for storing GMT date format.
+     */
+    private static ThreadLocal<DateFormat>
+        RFC1123_GMT_DATE_TIME_FORMATTER = new ThreadLocal<DateFormat>() {
+        @Override
+        protected DateFormat initialValue() {
+            final DateFormat formatter = new SimpleDateFormat(RFC1123_PATTERN, LOCALE_US);
+            formatter.setTimeZone(GMT_ZONE);
+            return formatter;
+        }
+    };
+
     /**
      * Stores a reference to the GMT time zone.
      */
@@ -116,13 +130,22 @@ public final class Utility {
      * Used to create Json parsers and generators.
      */
     private static final JsonFactory jsonFactory = new JsonFactory();
-    
+
     /**
-     * A factory to create SAXParser instances.
+     * Thread local for SAXParser.
      */
-    private static final ThreadLocal<SAXParserFactory> saxParserFactory = new ThreadLocal<SAXParserFactory>() {
-        @Override public SAXParserFactory initialValue() {
-            return SAXParserFactory.newInstance();
+    private static final ThreadLocal<SAXParser> saxParserThreadLocal = new ThreadLocal<SAXParser>() {
+        SAXParserFactory factory;
+        @Override public SAXParser initialValue() {
+            factory = SAXParserFactory.newInstance();
+            factory.setNamespaceAware(true);
+            try {
+                return factory.newSAXParser();
+            } catch (SAXException e) {
+                throw new RuntimeException("Unable to create SAXParser", e);
+            } catch (ParserConfigurationException e) {
+                throw new RuntimeException("Check parser configuration", e);
+            }
         }
     };
 
@@ -567,21 +590,18 @@ public static String getGMTTime() {
 
     /**
      * Returns the GTM date/time String for the specified value using the RFC1123 pattern.
-     * 
+     *
      * @param date
      *            A <code>Date</code> object that represents the date to convert to GMT date/time in the RFC1123
      *            pattern.
-     * 
+     *
      * @return A <code>String</code> that represents the GMT date/time for the specified value using the RFC1123
      *         pattern.
      */
     public static String getGMTTime(final Date date) {
-        final DateFormat formatter = new SimpleDateFormat(RFC1123_PATTERN, LOCALE_US);
-        formatter.setTimeZone(GMT_ZONE);
-        return formatter.format(date);
+        return RFC1123_GMT_DATE_TIME_FORMATTER.get().format(date);
     }
 
-    
     /**
      * Returns the UTC date/time String for the specified value using Java's version of the ISO8601 pattern,
      * which is limited to millisecond precision.
@@ -637,9 +657,7 @@ public static JsonGenerator getJsonGenerator(OutputStream outStream) throws IOEx
      */
     public static JsonParser getJsonParser(final String jsonString) throws JsonParseException, IOException {
         JsonParser parser = jsonFactory.createParser(jsonString);
-
-        // allows handling of infinity, -infinity, and NaN for Doubles
-        return parser.enable(JsonParser.Feature.ALLOW_NON_NUMERIC_NUMBERS);
+        return setupJsonParser(parser);
     }
 
     /**
@@ -654,6 +672,20 @@ public static JsonParser getJsonParser(final String jsonString) throws JsonParse
      */
     public static JsonParser getJsonParser(final InputStream inStream) throws JsonParseException, IOException {
         JsonParser parser = jsonFactory.createParser(inStream);
+        return setupJsonParser(parser);
+    }
+
+    /**
+     * Returns a <code>JsonParser</code> This JsonParser will allow non-numeric numbers.
+     * @param parser
+     *      A <code>JsonParser</code> to setup.
+     * @return
+     *      A <code>JsonParser</code> with settings configured.
+     */
+    private static JsonParser setupJsonParser(final JsonParser parser) {
+        // IMPORTANT: DO NOT REMOVE!
+        // don't close the stream and allow it to be drained completely in ExecutionEngine to improve socket reuse
+        parser.disable(JsonParser.Feature.AUTO_CLOSE_SOURCE);
 
         // allows handling of infinity, -infinity, and NaN for Doubles
         return parser.enable(JsonParser.Feature.ALLOW_NON_NUMERIC_NUMBERS);
@@ -668,8 +700,9 @@ public static JsonParser getJsonParser(final InputStream inStream) throws JsonPa
      * @throws SAXException
      */
     public static SAXParser getSAXParser() throws ParserConfigurationException, SAXException {
-        saxParserFactory.get().setNamespaceAware(true);
-        return saxParserFactory.get().newSAXParser();
+        SAXParser parser = saxParserThreadLocal.get();
+        parser.reset(); //reset to original config
+        return parser;
     }
     
     /**
@@ -811,9 +844,7 @@ public static HashMap<String, String> parseAccountString(final String parseStrin
      *             If the specified string is invalid.
      */
     public static Date parseRFC1123DateFromStringInGMT(final String value) throws ParseException {
-        final DateFormat format = new SimpleDateFormat(RFC1123_PATTERN, Utility.LOCALE_US);
-        format.setTimeZone(GMT_ZONE);
-        return format.parse(value);
+        return RFC1123_GMT_DATE_TIME_FORMATTER.get().parse(value);
     }
 
     /**
@@ -1253,7 +1284,7 @@ public static StreamMd5AndLength writeToOutputStream(final InputStream sourceStr
             long writeLength, final boolean rewindSourceStream, final boolean calculateMD5, OperationContext opContext,
             final RequestOptions options, final Boolean shouldFlush) throws IOException, StorageException {
         return writeToOutputStream(sourceStream, outStream, writeLength, rewindSourceStream, calculateMD5, opContext,
-                options, shouldFlush, null /*StorageRequest*/);
+                options, shouldFlush, null /*StorageRequest*/, null /* descriptor */);
     }
 
     /**
@@ -1280,6 +1311,11 @@ public static StreamMd5AndLength writeToOutputStream(final InputStream sourceStr
      *            maximum execution time.
      * @param request
      *            Used by download resume to set currentRequestByteCount on the request. Otherwise, null is always used.
+     * @param descriptor
+     *                A {@Link StreamMd5AndLength} object to append to in the case of recovery action or null if this is not called
+     *                from a recovery. This value needs to be passed for recovery in case part of the body has already been read,
+     *                the recovery will attempt to download the remaining bytes but will do MD5 validation on the originally
+     *                requested range size.
      * @return A {@link StreamMd5AndLength} object that contains the output stream length, and optionally the MD5 hash.
      * 
      * @throws IOException
@@ -1289,24 +1325,28 @@ public static StreamMd5AndLength writeToOutputStream(final InputStream sourceStr
      */
     public static StreamMd5AndLength writeToOutputStream(final InputStream sourceStream, final OutputStream outStream,
             long writeLength, final boolean rewindSourceStream, final boolean calculateMD5, OperationContext opContext,
-            final RequestOptions options, final Boolean shouldFlush, StorageRequest<?, ?, Integer> request)
+            final RequestOptions options, final Boolean shouldFlush, StorageRequest<?, ?, Integer> request, StreamMd5AndLength descriptor)
             throws IOException, StorageException {
         if (rewindSourceStream && sourceStream.markSupported()) {
             sourceStream.reset();
             sourceStream.mark(Constants.MAX_MARK_LENGTH);
         }
 
-        final StreamMd5AndLength retVal = new StreamMd5AndLength();
-
-        if (calculateMD5) {
-            try {
-                retVal.setDigest(MessageDigest.getInstance("MD5"));
-            }
-            catch (final NoSuchAlgorithmException e) {
-                // This wont happen, throw fatal.
-                throw Utility.generateNewUnexpectedStorageException(e);
+        if (descriptor == null) {
+            descriptor = new StreamMd5AndLength();
+            if (calculateMD5) {
+                try {
+                    descriptor.setDigest(MessageDigest.getInstance("MD5"));
+                }
+                catch (final NoSuchAlgorithmException e) {
+                    // This wont happen, throw fatal.
+                    throw Utility.generateNewUnexpectedStorageException(e);
+                }
             }
         }
+        else {
+            descriptor.setMd5(null);
+        }
 
         if (writeLength < 0) {
             writeLength = Long.MAX_VALUE;
@@ -1330,25 +1370,26 @@ public static StreamMd5AndLength writeToOutputStream(final InputStream sourceStr
             }
 
             if (calculateMD5) {
-                retVal.getDigest().update(retrievedBuff, 0, count);
+                descriptor.getDigest().update(retrievedBuff, 0, count);
             }
 
-            retVal.setLength(retVal.getLength() + count);
-            retVal.setCurrentOperationByteCount(retVal.getCurrentOperationByteCount() + count);
+            descriptor.setLength(descriptor.getLength() + count);
+            descriptor.setCurrentOperationByteCount(descriptor.getCurrentOperationByteCount() + count);
 
             if (request != null) {
                 request.setCurrentRequestByteCount(request.getCurrentRequestByteCount() + count);
+                request.setCurrentDescriptor(descriptor);
             }
 
-            nextCopy = (int) Math.min(retrievedBuff.length, writeLength - retVal.getLength());
+            nextCopy = (int) Math.min(retrievedBuff.length, writeLength - descriptor.getLength());
             count = sourceStream.read(retrievedBuff, 0, nextCopy);
         }
 
         if (outStream != null && shouldFlush) {
             outStream.flush();
         }
 
-        return retVal;
+        return descriptor;
     }
 
     /**
diff --git a/microsoft-azure-storage/src/com/microsoft/azure/storage/file/CloudFile.java b/microsoft-azure-storage/src/com/microsoft/azure/storage/file/CloudFile.java
diff --git a/pom.xml b/pom.xml

Original file line number	Diff line number	Diff line change
`@@ -145,7 +145,7 @@ public void write(byte[] data, int offset, int length) throws IOException {`
`145`	`145`
`146`	`146`	`@Override`
`147`	`147`	`public void write(InputStream sourceStream, long writeLength) throws IOException, StorageException {`
`148`		`- Utility.writeToOutputStream(sourceStream, this, writeLength, false, false, this.opContext, this.options, false);`
	`148`	`+ Utility.writeToOutputStream(sourceStream, this, writeLength, false, false, this.opContext, this.options, false);`
`149`	`149`	`}`
`150`	`150`
`151`	`151`	`@Override`