You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ma...@apache.org on 2019/12/11 21:18:02 UTC

svn commit: r1871195 - in /jackrabbit/oak/trunk: oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/ oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/ oak-blob-plugins/s...

Author: mattryan
Date: Wed Dec 11 21:18:01 2019
New Revision: 1871195

URL: http://svn.apache.org/viewvc?rev=1871195&view=rev
Log:
OAK-8104: Update to Azure SDK v8.6.0 to fix signed URI C-D headers.

The purpose of this commit is to fix Content-Disposition header support for signed download URIs.  The filename* portion of the header specification was disabled in OAK-8013 due to a bug in the Azure SDK pre-8.5.0.  This commit adds filename* portion support back into Oak, which addresses the following issues:
- OAK-8104 - Fix the Content-Disposition header correctly (required upgrading the Azure SDK version to 8.6.0)
- OAK-8607 - Undo the workarounds implemented in OAK-8013.

Modified:
    jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
    jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptions.java
    jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
    jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptionsTest.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/binary/BinaryAccessIT.java
    jackrabbit/oak/trunk/oak-parent/pom.xml

Modified: jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java (original)
+++ jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java Wed Dec 11 21:18:01 2019
@@ -1104,7 +1104,10 @@ public class AzureBlobStoreBackend exten
                                     null) :
                             blob.generateSharedAccessSignature(policy,
                                     optionalHeaders,
-                                    null);
+                                    null,
+                                    null,
+                                    null,
+                                    true);
             // Shared access signature is returned encoded already.
 
             String uriString = String.format("https://%s/%s/%s?%s",

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptions.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptions.java?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptions.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptions.java Wed Dec 11 21:18:01 2019
@@ -138,7 +138,7 @@ public class DataRecordDownloadOptions {
                 if (Strings.isNullOrEmpty(dispositionType)) {
                     dispositionType = DISPOSITION_TYPE_INLINE;
                 }
-                contentDispositionHeader = formatContentDispositionHeader(dispositionType, fileName, null);
+                contentDispositionHeader = formatContentDispositionHeader(dispositionType, fileName, rfc8187Encode(fileName));
             }
             else if (DISPOSITION_TYPE_ATTACHMENT.equals(this.dispositionType)) {
                 contentDispositionHeader = DISPOSITION_TYPE_ATTACHMENT;

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java Wed Dec 11 21:18:01 2019
@@ -200,19 +200,10 @@ public abstract class AbstractDataRecord
             assertEquals(200, conn.getResponseCode());
 
             assertEquals(mimeType, conn.getHeaderField("Content-Type"));
-//            This proper behavior is disabled due to https://github.com/Azure/azure-sdk-for-java/issues/2900
-//            (see also https://issues.apache.org/jira/browse/OAK-8013).  We can re-enable the full test
-//            once the issue is resolved.  -MR
-//            assertEquals(
-//                    String.format("%s; filename=\"%s\"; filename*=UTF-8''%s",
-//                            dispositionType, fileName,
-//                            new String(encodedFileName.getBytes(StandardCharsets.UTF_8))
-//                    ),
-//                    conn.getHeaderField("Content-Disposition")
-//            );
             assertEquals(
-                    String.format("%s; filename=\"%s\"",
-                            dispositionType, fileName
+                    String.format("%s; filename=\"%s\"; filename*=UTF-8''%s",
+                            dispositionType, fileName,
+                            encodedFileName
                     ),
                     conn.getHeaderField("Content-Disposition")
             );

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptionsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptionsTest.java?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptionsTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/DataRecordDownloadOptionsTest.java Wed Dec 11 21:18:01 2019
@@ -24,8 +24,6 @@ import static org.junit.Assert.assertNot
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
-import java.nio.charset.StandardCharsets;
-
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
@@ -38,7 +36,9 @@ public class DataRecordDownloadOptionsTe
     private static final String CHARACTER_ENCODING_UTF_8 = "utf-8";
     private static final String CHARACTER_ENCODING_ISO_8859_1 = "ISO-8859-1";
     private static final String FILE_NAME_IMAGE = "amazing summer sunset.png";
+    private static final String ENCODED_FILE_NAME_IMAGE = "amazing%20summer%20sunset.png";
     private static final String FILE_NAME_TEXT = "journal_entry_01-01-2000.txt";
+    private static final String ENCODED_FILE_NAME_TEXT = FILE_NAME_TEXT;
     private static final String DISPOSITION_TYPE_INLINE = "inline";
     private static final String DISPOSITION_TYPE_ATTACHMENT = "attachment";
 
@@ -118,7 +118,7 @@ public class DataRecordDownloadOptionsTe
                 );
     }
 
-    private String getContentDispositionHeader(String fileName, String dispositionType) {
+    private String getContentDispositionHeader(String fileName, String encodedFileName, String dispositionType) {
         if (Strings.isNullOrEmpty(fileName)) {
             if (dispositionType.equals(DISPOSITION_TYPE_ATTACHMENT)) {
                 return DISPOSITION_TYPE_ATTACHMENT;
@@ -129,13 +129,9 @@ public class DataRecordDownloadOptionsTe
         if (Strings.isNullOrEmpty(dispositionType)) {
             dispositionType = DISPOSITION_TYPE_INLINE;
         }
-        String fileNameStar = new String(fileName.getBytes(StandardCharsets.UTF_8));
-//        This proper behavior is disabled due to https://github.com/Azure/azure-sdk-for-java/issues/2900
-//        (see also https://issues.apache.org/jira/browse/OAK-8013).  We can re-enable the full test
-//        once the issue is resolved.  -MR
-//        return String.format("%s; filename=\"%s\"; filename*=UTF-8''%s",
-//                dispositionType, fileName, fileNameStar);
-        return String.format("%s; filename=\"%s\"", dispositionType, fileName);
+
+        return String.format("%s; filename=\"%s\"; filename*=UTF-8''%s",
+                dispositionType, fileName, encodedFileName);
     }
 
     @Test
@@ -223,7 +219,9 @@ public class DataRecordDownloadOptionsTe
             for (String dispositionType : Lists.newArrayList(DISPOSITION_TYPE_INLINE, DISPOSITION_TYPE_ATTACHMENT)) {
                 verifyContentDispositionHeader(
                         getOptions(null, null, fileName, dispositionType),
-                        getContentDispositionHeader(fileName, dispositionType)
+                        getContentDispositionHeader(fileName,
+                                fileName.equals(FILE_NAME_IMAGE) ? ENCODED_FILE_NAME_IMAGE : ENCODED_FILE_NAME_TEXT,
+                                dispositionType)
                 );
             }
         }
@@ -234,7 +232,7 @@ public class DataRecordDownloadOptionsTe
         // Ensures that the default disposition type is "inline"
         verifyContentDispositionHeader(
                 getOptions(null, null, FILE_NAME_IMAGE, null),
-                getContentDispositionHeader(FILE_NAME_IMAGE, DISPOSITION_TYPE_INLINE)
+                getContentDispositionHeader(FILE_NAME_IMAGE, ENCODED_FILE_NAME_IMAGE, DISPOSITION_TYPE_INLINE)
         );
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/binary/BinaryAccessIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/binary/BinaryAccessIT.java?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/binary/BinaryAccessIT.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/binary/BinaryAccessIT.java Wed Dec 11 21:18:01 2019
@@ -33,16 +33,18 @@ import static org.junit.Assert.assertNot
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+
 import java.net.HttpURLConnection;
 import java.net.URI;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
+
 import javax.jcr.AccessDeniedException;
 import javax.jcr.Binary;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 
+import com.google.common.collect.Iterables;
 import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.JackrabbitValueFactory;
@@ -62,8 +64,6 @@ import org.junit.runners.Parameterized;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.Iterables;
-
 /**
  * Integration test for direct binary GET/PUT via HTTP, that requires a fully working data store
  * (such as S3) for each {@link AbstractBinaryAccessIT#dataStoreFixtures() configured fixture}.
@@ -367,6 +367,7 @@ public class BinaryAccessIT extends Abst
         Binary binary = storeBinaryAndRetrieve(getAdminSession(), FILE_PATH, content);
 
         String expectedName = "beautiful landscape.png";
+        String encodedName = "beautiful%20landscape.png";
         BinaryDownloadOptions downloadOptions = BinaryDownloadOptions
                 .builder()
                 .withFileName(expectedName)
@@ -376,20 +377,9 @@ public class BinaryAccessIT extends Abst
         HttpURLConnection conn = (HttpURLConnection) downloadURI.toURL().openConnection();
         String contentDisposition = conn.getHeaderField("Content-Disposition");
         assertNotNull(contentDisposition);
-        String encodedName = new String(expectedName.getBytes(StandardCharsets.UTF_8));
-        // This proper behavior is disabled due to
-        // https://github.com/Azure/azure-sdk-for-java/issues/2900
-        // (see also https://issues.apache.org/jira/browse/OAK-8013,
-        // https://issues.apache.org/jira/browse/OAK-8104, and
-        // https://issues.apache.org/jira/browse/OAK-8105).  We can re-enable
-        // the full test once the issue is resolved.  -MR
-//        assertEquals(
-//                String.format("inline; filename=\"%s\"; filename*=UTF-8''%s",
-//                        expectedName, encodedName),
-//                contentDisposition
-//        );
         assertEquals(
-                String.format("inline; filename=\"%s\"", expectedName),
+                String.format("inline; filename=\"%s\"; filename*=UTF-8''%s",
+                        expectedName, encodedName),
                 contentDisposition
         );
 
@@ -407,6 +397,7 @@ public class BinaryAccessIT extends Abst
         Binary binary = storeBinaryAndRetrieve(getAdminSession(), FILE_PATH, content);
 
         String expectedName = "beautiful landscape.png";
+        String encodedName = "beautiful%20landscape.png";
         BinaryDownloadOptions downloadOptions = BinaryDownloadOptions
                 .builder()
                 .withFileName(expectedName)
@@ -417,20 +408,9 @@ public class BinaryAccessIT extends Abst
         HttpURLConnection conn = (HttpURLConnection) downloadURI.toURL().openConnection();
         String contentDisposition = conn.getHeaderField("Content-Disposition");
         assertNotNull(contentDisposition);
-        String encodedName = new String(expectedName.getBytes(StandardCharsets.UTF_8));
-        // This proper behavior is disabled due to
-        // https://github.com/Azure/azure-sdk-for-java/issues/2900
-        // (see also https://issues.apache.org/jira/browse/OAK-8013,
-        // https://issues.apache.org/jira/browse/OAK-8104, and
-        // https://issues.apache.org/jira/browse/OAK-8105).  We can re-enable
-        // the full test once the issue is resolved.  -MR
-//        assertEquals(
-//                String.format("attachment; filename=\"%s\"; filename*=UTF-8''%s",
-//                        expectedName, encodedName),
-//                contentDisposition
-//        );
         assertEquals(
-                String.format("attachment; filename=\"%s\"", expectedName),
+                String.format("attachment; filename=\"%s\"; filename*=UTF-8''%s",
+                        expectedName, encodedName),
                 contentDisposition
         );
 
@@ -487,6 +467,7 @@ public class BinaryAccessIT extends Abst
         String expectedMediaType = "image/png";
         String expectedCharacterEncoding = "utf-8";
         String expectedName = "beautiful landscape.png";
+        String encodedName = "beautiful%20landscape.png";
         BinaryDownloadOptions downloadOptions = BinaryDownloadOptions
                 .builder()
                 .withMediaType(expectedMediaType)
@@ -505,20 +486,9 @@ public class BinaryAccessIT extends Abst
 
         String contentDisposition = conn.getHeaderField("Content-Disposition");
         assertNotNull(contentDisposition);
-        String encodedName = new String(expectedName.getBytes(StandardCharsets.UTF_8));
-        // This proper behavior is disabled due to
-        // https://github.com/Azure/azure-sdk-for-java/issues/2900
-        // (see also https://issues.apache.org/jira/browse/OAK-8013,
-        // https://issues.apache.org/jira/browse/OAK-8104, and
-        // https://issues.apache.org/jira/browse/OAK-8105).  We can re-enable
-        // the full test once the issue is resolved.  -MR
-//        assertEquals(
-//                String.format("attachment; filename=\"%s\"; filename*=UTF-8''%s",
-//                        expectedName, encodedName),
-//                contentDisposition
-//        );
         assertEquals(
-                String.format("attachment; filename=\"%s\"", expectedName),
+                String.format("attachment; filename=\"%s\"; filename*=UTF-8''%s",
+                        expectedName, encodedName),
                 contentDisposition
         );
 

Modified: jackrabbit/oak/trunk/oak-parent/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-parent/pom.xml?rev=1871195&r1=1871194&r2=1871195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-parent/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-parent/pom.xml Wed Dec 11 21:18:01 2019
@@ -712,7 +712,7 @@
       <dependency>
         <groupId>com.microsoft.azure</groupId>
         <artifactId>azure-storage</artifactId>
-        <version>8.4.0</version>
+        <version>8.6.0</version>
       </dependency>
       <dependency>
         <groupId>com.microsoft.azure</groupId>