-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Nio check access #18095
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.
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
Nio check access #18095
Changes from 11 commits
8d06f25
c0dc3dc
ffd7465
ba91298
1bab6e0
64e1a7b
e4d7198
22ecb7f
45c20a9
81b5ad2
bd31b45
82ddd75
5f16848
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| import java.net.HttpURLConnection; | ||
| import java.net.URI; | ||
| import java.nio.channels.SeekableByteChannel; | ||
| import java.nio.file.AccessDeniedException; | ||
| import java.nio.file.AccessMode; | ||
| import java.nio.file.CopyOption; | ||
| import java.nio.file.DirectoryNotEmptyException; | ||
|
|
@@ -373,7 +374,8 @@ public OutputStream newOutputStream(Path path, OpenOption... options) throws IOE | |
| // Write and truncate must be specified | ||
| if (!optionsList.contains(StandardOpenOption.WRITE) | ||
| || !optionsList.contains(StandardOpenOption.TRUNCATE_EXISTING)) { | ||
| throw new IllegalArgumentException("Write and TruncateExisting must be specified to open an OutputStream"); | ||
| throw LoggingUtility.logError(logger, | ||
| new IllegalArgumentException("Write and TruncateExisting must be specified to open an OutputStream")); | ||
| } | ||
|
|
||
| AzureResource resource = new AzureResource(path); | ||
|
|
@@ -785,11 +787,14 @@ public FileStore getFileStore(Path path) throws IOException { | |
| } | ||
|
|
||
| /** | ||
| * Unsupported. | ||
| * Checks the existence, and optionally the accessibility, of a file. | ||
| * <p> | ||
| * This method may only be used to check the existence of a file. It is not possible to determine the permissions | ||
| * granted to a given client, so if any mode argument is specified, an {@link UnsupportedOperationException} will be | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be UnsupportedOperationException or AccessDeniedException?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AccessDenied
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (per the jdk docs for this method: "the requested access would be denied or the access cannot be determined because the Java virtual machine has insufficient privileges or other reasons." I think it's reasonable to put this under access cannot be determined) |
||
| * thrown. | ||
| * | ||
| * @param path path | ||
| * @param accessModes accessMode | ||
| * @throws UnsupportedOperationException Operation is not supported. | ||
| * @param path the path to the file to check | ||
| * @param accessModes The access modes to check; may have zero elements | ||
| * @throws NoSuchFileException if a file does not exist | ||
| * @throws java.nio.file.AccessDeniedException the requested access would be denied or the access cannot be | ||
| * determined because the Java virtual machine has insufficient privileges or other reasons | ||
|
|
@@ -798,7 +803,23 @@ public FileStore getFileStore(Path path) throws IOException { | |
| */ | ||
| @Override | ||
| public void checkAccess(Path path, AccessMode... accessModes) throws IOException { | ||
| throw new UnsupportedOperationException(); | ||
| if (accessModes != null && accessModes.length != 0) { | ||
| throw LoggingUtility.logError(logger, new AccessDeniedException("The access cannot be determined.")); | ||
| } | ||
| AzurePath.ensureFileSystemOpen(path); | ||
|
|
||
| // Read attributes already wraps BlobStorageException in an IOException. | ||
| try { | ||
| readAttributes(path, BasicFileAttributes.class); | ||
| } catch (IOException e) { | ||
| Throwable cause = e.getCause(); | ||
| if (cause instanceof BlobStorageException | ||
| && BlobErrorCode.BLOB_NOT_FOUND.equals(((BlobStorageException) cause).getErrorCode())) { | ||
| throw LoggingUtility.logError(logger, new NoSuchFileException(path.toString())); | ||
| } else { | ||
| throw e; | ||
|
rickle-msft marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,15 +3,28 @@ | |
|
|
||
| package com.azure.storage.blob.nio | ||
|
|
||
| import com.azure.core.http.HttpHeaders | ||
| import com.azure.core.http.HttpMethod | ||
| import com.azure.core.http.HttpPipelineCallContext | ||
| import com.azure.core.http.HttpPipelineNextPolicy | ||
| import com.azure.core.http.HttpRequest | ||
| import com.azure.core.http.HttpResponse | ||
| import com.azure.core.http.policy.HttpPipelinePolicy | ||
| import com.azure.storage.blob.BlobClient | ||
| import com.azure.storage.blob.models.AccessTier | ||
| import com.azure.storage.blob.models.BlobErrorCode | ||
| import com.azure.storage.blob.models.BlobHttpHeaders | ||
| import com.azure.storage.blob.specialized.AppendBlobClient | ||
| import com.azure.storage.blob.specialized.BlockBlobClient | ||
| import reactor.core.publisher.Flux | ||
| import reactor.core.publisher.Mono | ||
| import spock.lang.Requires | ||
| import spock.lang.Unroll | ||
|
|
||
| import java.nio.ByteBuffer | ||
| import java.nio.charset.Charset | ||
| import java.nio.file.AccessDeniedException | ||
| import java.nio.file.AccessMode | ||
| import java.nio.file.DirectoryNotEmptyException | ||
| import java.nio.file.FileAlreadyExistsException | ||
| import java.nio.file.FileSystem | ||
|
|
@@ -1058,6 +1071,140 @@ class AzureFileSystemProviderTest extends APISpec { | |
| thrown(IOException) | ||
| } | ||
|
|
||
| def "CheckAccess"() { | ||
| setup: | ||
| def fs = createFS(config) | ||
| def path = fs.getPath(generateBlobName()) | ||
| def os = fs.provider().newOutputStream(path) | ||
| os.close() | ||
|
|
||
| when: | ||
| fs.provider().checkAccess(path) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could probably just do
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's functionally the same. If all I'm doing is check the call succeeds, I usually like to make that explicit by saying I don't want an exception. |
||
|
|
||
| then: | ||
| notThrown(Exception) | ||
| } | ||
|
|
||
| @Unroll | ||
| def "CheckAccess AccessDenied"() { | ||
| setup: | ||
| def fs = createFS(config) | ||
| def path = fs.getPath(generateBlobName()) | ||
| def os = fs.provider().newOutputStream(path) | ||
| os.close() | ||
|
|
||
| when: | ||
| fs.provider().checkAccess(path, mode) | ||
|
|
||
| then: | ||
| thrown(AccessDeniedException) | ||
|
|
||
| where: | ||
| mode | _ | ||
| AccessMode.READ | _ | ||
| AccessMode.WRITE | _ | ||
| AccessMode.EXECUTE | _ | ||
| } | ||
|
|
||
| def "CheckAccess IOException"() { | ||
| setup: | ||
| config = initializeConfigMap(new CheckAccessIoExceptionPolicy()) | ||
| def fs = createFS(config) | ||
| def path = fs.getPath(generateBlobName()) | ||
| def os = fs.provider().newOutputStream(path) | ||
| os.close() | ||
|
|
||
| when: | ||
| fs.provider().checkAccess(path) | ||
|
|
||
| then: | ||
| def e = thrown(IOException) | ||
| !(e instanceof NoSuchFileException) | ||
| } | ||
|
|
||
| class CheckAccessIoExceptionPolicy implements HttpPipelinePolicy { | ||
| @Override | ||
| Mono<HttpResponse> process(HttpPipelineCallContext httpPipelineCallContext, HttpPipelineNextPolicy httpPipelineNextPolicy) { | ||
| HttpRequest request = httpPipelineCallContext.getHttpRequest() | ||
| // GetProperties call to blob | ||
| if (request.getUrl().getPath().split("/").size() == 3 && request.getHttpMethod() == (HttpMethod.HEAD)) { | ||
| return Mono.just(new checkAccessIoExceptionResponse(request)) | ||
| } else { | ||
| return httpPipelineNextPolicy.process() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class checkAccessIoExceptionResponse extends HttpResponse { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: capitalize the C |
||
| protected checkAccessIoExceptionResponse(HttpRequest request) { | ||
| super(request) | ||
| } | ||
|
|
||
| @Override | ||
| int getStatusCode() { | ||
| return 403 | ||
| } | ||
|
|
||
| @Override | ||
| String getHeaderValue(String s) { | ||
| return request.getHeaders().getValue(s) | ||
| } | ||
|
|
||
| @Override | ||
| HttpHeaders getHeaders() { | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.put("x-ms-error-code", BlobErrorCode.AUTHORIZATION_FAILURE.toString()) | ||
| return headers | ||
| } | ||
|
|
||
| @Override | ||
| Flux<ByteBuffer> getBody() { | ||
| return Flux.just(ByteBuffer.allocate(0)) | ||
| } | ||
|
|
||
| @Override | ||
| Mono<byte[]> getBodyAsByteArray() { | ||
| return Mono.just(new byte[0]) | ||
| } | ||
|
|
||
| @Override | ||
| Mono<String> getBodyAsString() { | ||
| return Mono.just("") | ||
| } | ||
|
|
||
| @Override | ||
| Mono<String> getBodyAsString(Charset charset) { | ||
| return Mono.just("") | ||
| } | ||
| } | ||
|
|
||
| def "CheckAccess no file"() { | ||
| setup: | ||
| def fs = createFS(config) | ||
| def path = fs.getPath(generateBlobName()) | ||
|
|
||
| when: | ||
| fs.provider().checkAccess(path) | ||
|
|
||
| then: | ||
| thrown(NoSuchFileException) | ||
| } | ||
|
|
||
| def "CheckAccess fs closed"() { | ||
| setup: | ||
| def fs = createFS(config) | ||
| def path = fs.getPath(generateBlobName()) | ||
| def os = fs.provider().newOutputStream(path) | ||
| os.close() | ||
|
|
||
| when: | ||
| fs.close() | ||
| fs.provider().checkAccess(path) | ||
|
|
||
| then: | ||
| thrown(IOException) | ||
| } | ||
|
|
||
| @Unroll | ||
| def "GetAttributeView"() { | ||
| setup: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.