Russole commented on PR #9371:
URL: https://github.com/apache/ozone/pull/9371#issuecomment-3589292264
> Few other scenarios are also possible, so do you have idea what will be
the behaviour in the below scenarios?:
>
> * What if header exists but is empty and no body is there?
>
> ```
> example:
> when(mockHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL))
> .thenReturn(""); //empty
> Response resp = bucketEndpoint.put(bucketName, "acl", null);
> ```
>
> * Or what if header exists but has white space only and no body?
>
> ```
> example:
> when(mockHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL))
> .thenReturn(" "); // whitespace
> Response resp = bucketEndpoint.put(bucketName, "acl", null);
> ```
>
> So in the above two case what should happen, create bucket should pass or
fail ? And according do testing of these two scenarios as well.
Thanks for pointing out these additional edge cases.
I checked the current implementation of BucketEndpoint.putAcl() (in
particular getAndConvertAclOnBucket / getAndConvertAclOnVolume) and added tests
to document the behavior for both scenarios:
* Header exists but is empty (""), no body
```java
when(mockHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL))
.thenReturn("");
Response resp = bucketEndpoint.put(bucketName, "acl", null);
```
In this case, StringUtils.isEmpty(value) returns true, so the helper returns
an empty ACL list without throwing. The PUT ?acl call succeeds with HTTP 200
and effectively behaves as “no grants provided”.
This is covered by:
```java
@Test
public void testPutAclWithEmptyGrantHeaderValue() throws Exception {
assertEquals(200, resp.getStatus());
}
```
* Header exists but has only whitespace (" "), no body
```java
when(mockHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL))
.thenReturn(" ");
Response resp = bucketEndpoint.put(bucketName, "acl", null);
```
Here StringUtils.isEmpty(value) is false, so we attempt to parse the value.
Splitting on "=" produces a single part (part.length != 2), which triggers:
```java
throw newError(S3ErrorTable.INVALID_ARGUMENT, acl);
```
So this scenario fails with OS3Exception(INVALID_ARGUMENT, 400).
This is covered by:
```java
@Test
public void testPutAclWithWhitespaceGrantHeaderValue() throws Exception {
OS3Exception ex = assertThrows(OS3Exception.class,
() -> bucketEndpoint.put(bucketName, "acl", null));
assertEquals(INVALID_ARGUMENT.getCode(), ex.getCode());
assertEquals(400, ex.getHttpCode());
}
```
These two tests make the current behavior explicit and should help prevent
regressions around header parsing for PUT ?acl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
