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.

Sign up for GitHub

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

Fixed double decoding in getBlobName #22825

Merged
merged 2 commits into from
Jul 8, 2021
Merged

Fixed double decoding in getBlobName #22825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af1ef65
Fixed double decoding in getBlobName
rickle-msft Jul 7, 2021
55f5d3f
Pr feedback and reformat
rickle-msft Jul 7, 2021
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
2 changes: 1 addition & 1 deletion ...re-storage-blob/src/main/java/com/azure/storage/blob/specialized/BlobAsyncClientBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ final BlobContainerClientBuilder getContainerClientBuilder() {
* @return The decoded name of the blob.
*/
public final String getBlobName() {
return (blobName == null) ? null : Utility.urlDecode(blobName);
return blobName; // The blob name is decoded when the client is constructor
}

/**
Expand Down
125 changes: 64 additions & 61 deletions sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobAPITest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@

package com.azure.storage.blob

import com.azure.core.http.HttpPipelineCallContext
import com.azure.core.http.HttpPipelineNextPolicy
import com.azure.core.http.HttpResponse

import com.azure.core.http.RequestConditions
import com.azure.core.http.policy.HttpPipelinePolicy
import com.azure.core.util.BinaryData
import com.azure.core.util.CoreUtils
import com.azure.core.util.polling.LongRunningOperationStatus
Expand Down Expand Up @@ -45,20 +42,17 @@ import com.azure.storage.blob.sas.BlobSasPermission
import com.azure.storage.blob.sas.BlobServiceSasSignatureValues
import com.azure.storage.blob.specialized.BlobClientBase
import com.azure.storage.blob.specialized.SpecializedBlobClientBuilder
import com.azure.storage.common.Utility
import com.azure.storage.common.implementation.Constants
import com.azure.storage.common.test.shared.extensions.LiveOnly
import com.azure.storage.common.test.shared.extensions.PlaybackOnly
import com.azure.storage.common.test.shared.extensions.RequiredServiceVersion
import com.azure.storage.common.test.shared.policy.MockFailureResponsePolicy
import com.azure.storage.common.test.shared.policy.MockRetryRangeResponsePolicy
import reactor.core.Exceptions
import reactor.core.publisher.Flux
import reactor.core.publisher.Hooks
import reactor.core.publisher.Mono
import reactor.test.StepVerifier
import spock.lang.IgnoreIf
import spock.lang.Requires
import spock.lang.ResourceLock
import spock.lang.Unroll
import spock.lang.Ignore

Expand Down Expand Up @@ -608,7 +602,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

@Unroll
Expand Down Expand Up @@ -636,7 +630,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

@Unroll
Expand Down Expand Up @@ -664,7 +658,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Download md5"() {
Expand Down Expand Up @@ -1009,12 +1003,12 @@ class BlobAPITest extends APISpec {
send off parallel requests with invalid ranges.
*/
where:
range | _
range | _
new BlobRange(0, data.defaultDataSize) | _ // Exact count
new BlobRange(1, data.defaultDataSize - 1 as Long) | _ // Offset and exact count
new BlobRange(3, 2) | _ // Narrow range in middle
new BlobRange(3, 2) | _ // Narrow range in middle
new BlobRange(0, data.defaultDataSize - 1 as Long) | _ // Count that is less than total
new BlobRange(0, 10 * 1024) | _ // Count much larger than remaining data
new BlobRange(0, 10 * 1024) | _ // Count much larger than remaining data
}

/*
Expand Down Expand Up @@ -1261,7 +1255,8 @@ class BlobAPITest extends APISpec {
}

@Unroll
@Ignore("Very large data sizes.") /* Enable once we have ability to run large resource heavy tests in CI. */
@Ignore("Very large data sizes.")
/* Enable once we have ability to run large resource heavy tests in CI. */
def "Download to file blockSize"() {
def file = getRandomFile(sizeOfData)
bc.uploadFromFile(file.toPath().toString(), true)
Expand All @@ -1279,10 +1274,10 @@ class BlobAPITest extends APISpec {
notThrown(BlobStorageException)

where:
sizeOfData | downloadBlockSize || _
5000 * Constants.MB | 5000 * Constants.MB || _ /* This was the default before. */
6000 * Constants.MB | 6000 * Constants.MB || _ /* Trying to see if we can set it to a number greater than previous default. */
6000 * Constants.MB | 5100 * Constants.MB || _ /* Testing chunking with a large size */
sizeOfData | downloadBlockSize || _
5000 * Constants.MB | 5000 * Constants.MB || _ /* This was the default before. */
6000 * Constants.MB | 6000 * Constants.MB || _ /* Trying to see if we can set it to a number greater than previous default. */
6000 * Constants.MB | 5100 * Constants.MB || _ /* Testing chunking with a large size */
}


Expand Down Expand Up @@ -1388,7 +1383,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

/*
Expand Down Expand Up @@ -1494,8 +1489,8 @@ class BlobAPITest extends APISpec {
cacheControl, contentDisposition, contentEncoding, contentLanguage, contentMD5, contentType)

where:
cacheControl | contentDisposition | contentEncoding | contentLanguage | contentMD5 | contentType
null | null | null | null | null | null
cacheControl | contentDisposition | contentEncoding | contentLanguage | contentMD5 | contentType
null | null | null | null | null | null
"control" | "disposition" | "encoding" | "language" | Base64.getEncoder().encode(MessageDigest.getInstance("MD5").digest(data.defaultBytes)) | "type"
}

Expand Down Expand Up @@ -1556,7 +1551,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Set HTTP headers error"() {
Expand Down Expand Up @@ -1609,11 +1604,11 @@ class BlobAPITest extends APISpec {
bc.getProperties().getMetadata() == metadata

where:
key1 | value1 | key2 | value2 || statusCode
null | null | null | null || 200
"foo" | "bar" | "fizz" | "buzz" || 200
"i0" | "a" | "i_" | "a" || 200 /* Test culture sensitive word sort */
"foo" | "bar0, bar1" | null | null || 200 /* Test comma separated values */
key1 | value1 | key2 | value2 || statusCode
null | null | null | null || 200
"foo" | "bar" | "fizz" | "buzz" || 200
"i0" | "a" | "i_" | "a" || 200 /* Test culture sensitive word sort */
"foo" | "bar0, bar1" | null | null || 200 /* Test comma separated values */
}

@RequiredServiceVersion(clazz = BlobServiceVersion.class, min = "V2019_12_12")
Expand Down Expand Up @@ -1673,7 +1668,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

@Unroll
Expand All @@ -1692,11 +1687,11 @@ class BlobAPITest extends APISpec {
// On Playback, the framework will throw Exceptions.ReactiveException.

where:
key | value || _
" foo" | "bar" || _ // Leading whitespace key
"foo " | "bar" || _ // Trailing whitespace key
"foo" | " bar" || _ // Leading whitespace value
"foo" | "bar " || _ // Trailing whitespace value
key | value || _
" foo" | "bar" || _ // Leading whitespace key
"foo " | "bar" || _ // Trailing whitespace key
"foo" | " bar" || _ // Leading whitespace value
"foo" | "bar " || _ // Trailing whitespace value
}

def "Set metadata error"() {
Expand Down Expand Up @@ -2028,7 +2023,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Snapshot error"() {
Expand Down Expand Up @@ -2223,13 +2218,13 @@ class BlobAPITest extends APISpec {
response.getStatus() == LongRunningOperationStatus.SUCCESSFULLY_COMPLETED

where:
modified | unmodified | match | noneMatch | tags
null | null | null | null | null
oldDate | null | null | null | null
null | newDate | null | null | null
null | null | receivedEtag | null | null
null | null | null | garbageEtag | null
null | null | null | null | "\"foo\" = 'bar'"
modified | unmodified | match | noneMatch | tags
null | null | null | null | null
oldDate | null | null | null | null
null | newDate | null | null | null
null | null | receivedEtag | null | null
null | null | null | garbageEtag | null
null | null | null | null | "\"foo\" = 'bar'"
}

@Unroll
Expand All @@ -2252,12 +2247,12 @@ class BlobAPITest extends APISpec {
thrown(BlobStorageException)

where:
modified | unmodified | match | noneMatch | tags
newDate | null | null | null | null
null | oldDate | null | null | null
null | null | garbageEtag | null | null
null | null | null | receivedEtag | null
null | null | null | null | "\"notfoo\" = 'notbar'"
modified | unmodified | match | noneMatch | tags
newDate | null | null | null | null
null | oldDate | null | null | null
null | null | garbageEtag | null | null
null | null | null | receivedEtag | null
null | null | null | null | "\"notfoo\" = 'notbar'"
}

@RequiredServiceVersion(clazz = BlobServiceVersion.class, min = "V2019_12_12")
Expand Down Expand Up @@ -2325,7 +2320,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Abort copy lease fail"() {
Expand Down Expand Up @@ -2641,7 +2636,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Sync copy error"() {
Expand Down Expand Up @@ -2749,7 +2744,7 @@ class BlobAPITest extends APISpec {
null | null | garbageEtag | null | null | null
null | null | null | receivedEtag | null | null
null | null | null | null | garbageLeaseID | null
null | null | null | null | null | "\"notfoo\" = 'notbar'"
null | null | null | null | null | "\"notfoo\" = 'notbar'"
}

def "Blob delete error"() {
Expand Down Expand Up @@ -3094,8 +3089,16 @@ class BlobAPITest extends APISpec {
}

def "Get Blob Name"() {
setup:
bc = cc.getBlobClient(inputName)

expect:
blobName == bc.getBlobName()
expectedOutputName == bc.getBlobName()

where:
inputName | expectedOutputName
"blobName" | "blobName" // standard names should be preserved
Utility.urlEncode("dir1/a%20b.txt") | "dir1/a%20b.txt" // encoded names should be decoded (not double decoded
}

def "Get Blob Name and Build Client"() {
Expand Down Expand Up @@ -3143,16 +3146,16 @@ class BlobAPITest extends APISpec {
thrown(IllegalArgumentException)
}

@IgnoreIf( { getEnv().serviceVersion != null } )
@IgnoreIf({ getEnv().serviceVersion != null })
// This tests the policy is in the right place because if it were added per retry, it would be after the credentials and auth would fail because we changed a signed header.
def "Per call policy"() {
bc = getBlobClient(env.primaryAccount.credential, bc.getBlobUrl(), getPerCallVersionPolicy())
def "Per call policy"() {
bc = getBlobClient(env.primaryAccount.credential, bc.getBlobUrl(), getPerCallVersionPolicy())

when:
def response = bc.getPropertiesWithResponse(null, null, null)
when:
def response = bc.getPropertiesWithResponse(null, null, null)

then:
notThrown(BlobStorageException)
response.getHeaders().getValue("x-ms-version") == "2017-11-09"
}
then:
notThrown(BlobStorageException)
response.getHeaders().getValue("x-ms-version") == "2017-11-09"
}
}
51 changes: 51 additions & 0 deletions ...rage/azure-storage-blob/src/test/resources/session-records/BlobAPITestGetBlobName[0].json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{
"networkCallRecords" : [ {
"Method" : "PUT",
"Uri" : "https://REDACTED.blob.core.windows.net/a79a70b90a79a70b9bb761329e26ab48c467e4b068d6?restype=container",
"Headers" : {
"x-ms-version" : "2020-10-02",
"User-Agent" : "azsdk-java-azure-storage-blob/12.13.0-beta.1 (11.0.7; Windows 10; 10.0)",
"x-ms-client-request-id" : "07ba1380-ed23-47ec-9317-b0e46f6880b2"
},
"Response" : {
"content-length" : "0",
"x-ms-version" : "2020-10-02",
"Server" : "Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0",
"eTag" : "0x8D94175691E2DFC",
"Last-Modified" : "Wed, 07 Jul 2021 18:31:18 GMT",
"retry-after" : "0",
"StatusCode" : "201",
"x-ms-request-id" : "7fa854b0-e01e-0061-3b5e-73f981000000",
"x-ms-client-request-id" : "07ba1380-ed23-47ec-9317-b0e46f6880b2",
"Date" : "Wed, 07 Jul 2021 18:31:17 GMT"
},
"Exception" : null
}, {
"Method" : "PUT",
"Uri" : "https://REDACTED.blob.core.windows.net/a79a70b90a79a70b9bb761329e26ab48c467e4b068d6/a79a70b91a79a70b9bb729766da6ffe92d0ad41a5a47",
"Headers" : {
"x-ms-version" : "2020-10-02",
"User-Agent" : "azsdk-java-azure-storage-blob/12.13.0-beta.1 (11.0.7; Windows 10; 10.0)",
"x-ms-client-request-id" : "eb8a1b98-15ae-431d-b0a6-826bdae0105b",
"Content-Type" : "application/octet-stream"
},
"Response" : {
"content-length" : "0",
"x-ms-version" : "2020-10-02",
"Server" : "Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0",
"x-ms-content-crc64" : "6RYQPwaVsyQ=",
"Last-Modified" : "Wed, 07 Jul 2021 18:31:18 GMT",
"x-ms-version-id" : "2021-07-07T18:31:18.5446124Z",
"retry-after" : "0",
"StatusCode" : "201",
"x-ms-request-server-encrypted" : "true",
"Date" : "Wed, 07 Jul 2021 18:31:17 GMT",
"Content-MD5" : "wh+Wm18D0z1D4E+PE252gg==",
"eTag" : "0x8D94175696E38EC",
"x-ms-request-id" : "5630357b-701e-003e-565e-734dbd000000",
"x-ms-client-request-id" : "eb8a1b98-15ae-431d-b0a6-826bdae0105b"
},
"Exception" : null
} ],
"variables" : [ "a79a70b90a79a70b9bb761329e26ab48c467e4b068d6", "a79a70b91a79a70b9bb729766da6ffe92d0ad41a5a47" ]
}
Loading