You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/11/12 06:27:21 UTC
[GitHub] [ozone] k5342 opened a new pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
k5342 opened a new pull request #2834:
URL: https://github.com/apache/ozone/pull/2834
## What changes were proposed in this pull request?
Add an url-encode feature for key and prefix to fix an S3G issue that cannot handle keys named like 'a+b+c'.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5975
## How was this patch tested?
WIP & I'll add them later. I planned to modify TestBucketList.java.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#issuecomment-997168823
> BTW, I noticed `S3Utils.urlEncode` function has introduced recently #2912. Could we use that method instead of URLEncoder?
Sure.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] k5342 commented on a change in pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
k5342 commented on a change in pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#discussion_r770643682
##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/ObjectKeyNameAdapter.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.s3.commontypes;
+
+import javax.xml.bind.annotation.adapters.XmlAdapter;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+
+/**
+ * A converter to convert raw-String to S3 compliant object key name.
+ * ref: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
+ */
+public class ObjectKeyNameAdapter extends XmlAdapter<String, String> {
+
+ private static final Charset UTF_8 = StandardCharsets.UTF_8;
+
+ public ObjectKeyNameAdapter() {
+ }
+
+ @Override
+ public String unmarshal(String s) throws Exception {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public String marshal(String s) throws Exception {
+ return urlEncode(s);
+ }
+
+ private static String urlEncode(String str) {
+ try {
+ return URLEncoder.encode(str, UTF_8.name())
+ .replaceAll("%2F", "/");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
Review comment:
Thank you for a review! Indeed, there is no need to convert the exception in this context. I will fix it not to convert.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] k5342 commented on pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
k5342 commented on pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#issuecomment-996590331
I've added a test for handling of special characters.
BTW, I noticed `S3Utils.urlEncode` function has introduced recently https://github.com/apache/ozone/pull/2912. Could we use that method instead of URLEncoder?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] k5342 commented on pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
k5342 commented on pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#issuecomment-998572120
Thank you for comment. I replaced my PR by S3Utils version. Sorry for rebase.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 merged pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2834:
URL: https://github.com/apache/ozone/pull/2834
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#discussion_r769451291
##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/ObjectKeyNameAdapter.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.s3.commontypes;
+
+import javax.xml.bind.annotation.adapters.XmlAdapter;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+
+/**
+ * A converter to convert raw-String to S3 compliant object key name.
+ * ref: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
+ */
+public class ObjectKeyNameAdapter extends XmlAdapter<String, String> {
+
+ private static final Charset UTF_8 = StandardCharsets.UTF_8;
+
+ public ObjectKeyNameAdapter() {
+ }
Review comment:
Nit: matches default constructor, can it be omitted?
##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/ObjectKeyNameAdapter.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.s3.commontypes;
+
+import javax.xml.bind.annotation.adapters.XmlAdapter;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+
+/**
+ * A converter to convert raw-String to S3 compliant object key name.
+ * ref: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
+ */
+public class ObjectKeyNameAdapter extends XmlAdapter<String, String> {
+
+ private static final Charset UTF_8 = StandardCharsets.UTF_8;
Review comment:
Nit: do we need a local constant? I think static import of `StandardCharsets.UTF_8` would be enough.
##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/commontypes/ObjectKeyNameAdapter.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.s3.commontypes;
+
+import javax.xml.bind.annotation.adapters.XmlAdapter;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+
+/**
+ * A converter to convert raw-String to S3 compliant object key name.
+ * ref: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
+ */
+public class ObjectKeyNameAdapter extends XmlAdapter<String, String> {
+
+ private static final Charset UTF_8 = StandardCharsets.UTF_8;
+
+ public ObjectKeyNameAdapter() {
+ }
+
+ @Override
+ public String unmarshal(String s) throws Exception {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public String marshal(String s) throws Exception {
+ return urlEncode(s);
+ }
+
+ private static String urlEncode(String str) {
+ try {
+ return URLEncoder.encode(str, UTF_8.name())
+ .replaceAll("%2F", "/");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
Review comment:
`marshal` allows throwing checked exceptions, too. Do we really need to convert to unchecked exception? (If so, Sonar would prefer some more specific exception type.)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] k5342 commented on pull request #2834: HDDS-5975. Serve url-encoded key and prefix name for ListObjectResponse
Posted by GitBox <gi...@apache.org>.
k5342 commented on pull request #2834:
URL: https://github.com/apache/ozone/pull/2834#issuecomment-971307388
I've attached small tests for ObjectKeyNameAdapter to validate encoding results. However, it currently does not cover about sanitizing on XML format such like `>` and `<` and integration test with ListBucketResult. Another test file TestBucketResponse may help.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org