rshkv commented on code in PR #12309:
URL: https://github.com/apache/iceberg/pull/12309#discussion_r1959986550
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java:
##########
@@ -108,16 +110,6 @@ public void testNamespaceUrlEncodeDecodeDoesNotAllowNull()
{
.withMessage("Invalid namespace: null");
}
- @Test
- @SuppressWarnings("checkstyle:AvoidEscapedUnicodeCharacters")
- public void testOAuth2URLEncoding() {
- // from OAuth2, RFC 6749 Appendix B.
- String utf8 = "\u0020\u0025\u0026\u002B\u00A3\u20AC";
- String expected = "+%25%26%2B%C2%A3%E2%82%AC";
-
- assertThat(RESTUtil.encodeString(utf8)).isEqualTo(expected);
- }
Review Comment:
This one I'm not sure 100% about but I think it's safe to remove.
The test is failing now because it asserts that the `\u0020` space character
becomes `+` as that's the example in [RFC 6949 Appendix
B](https://www.rfc-editor.org/rfc/rfc6749.html#appendix-B) which Ryan linked to.
I understand removing the test is fine because 1) the appendix describes how
to encode payloads instead of URLs and we cover payload encoding in the
`testOAuth2FormDataEncoding` test below. And 2), the space _can_ be
percent-encoded for something that expects `x-www-form-urlencoded` as evidenced
by `java.net.URLDecoder` decoding it. For OAuth specifically, there is also
[RFC 5849, 3.6](https://datatracker.ietf.org/doc/html/rfc5849#section-3.6):
> This specification defines the following method for percent-encoding
strings:
>
> ...
>
> This method is different from the encoding scheme used by the
"application/x-www-form-urlencoded" content-type (for example, it encodes space
characters as "%20" and not using the "+" character). It MAY be different from
the percent-encoding functions provided by web-development frameworks (e.g.,
encode different characters, use lowercase hexadecimal characters).
RFC 6949 is newer than 5849 but I'm linking to the above as evidence that
6949 was probably more to say "you should encode" than specifically "you should
encode space as `+`".
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
