You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "jkbkupczyk (via GitHub)" <gi...@apache.org> on 2023/07/01 10:35:56 UTC
[GitHub] [commons-net] jkbkupczyk opened a new pull request, #161: Add Base64 missing tests and documentation fixes
jkbkupczyk opened a new pull request, #161:
URL: https://github.com/apache/commons-net/pull/161
- Adds missing tests for Base64 decoding and encoding (example Base64 data comes from [Base64 - Wikipedia](https://en.wikipedia.org/wiki/Base64) and [RFC - 4648](https://datatracker.ietf.org/doc/html/rfc4648))
- Updates Base64 documentation to reflect code and fixes some typos
FYI @garydgregory @kinow
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] jkbkupczyk commented on a diff in pull request #161: Add Base64 missing tests and documentation fixes
Posted by "jkbkupczyk (via GitHub)" <gi...@apache.org>.
jkbkupczyk commented on code in PR #161:
URL: https://github.com/apache/commons-net/pull/161#discussion_r1249331794
##########
src/main/java/org/apache/commons/net/util/Base64.java:
##########
@@ -603,10 +603,10 @@ int avail() {
}
/**
- * Decodes a byte[] containing characters in the Base64 alphabet.
+ * Decodes a byte array containing characters in the Base64 alphabet.
*
* @param pArray A byte array containing Base64 character data
- * @return a byte array containing binary data
+ * @return a byte array containing binary data; will return {@code null} if provided byte array is null.
Review Comment:
fixed in 7c11889a03a3754a9abd6874cf8b2d06707d0cb9
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] jkbkupczyk commented on a diff in pull request #161: Add Base64 missing tests and documentation fixes
Posted by "jkbkupczyk (via GitHub)" <gi...@apache.org>.
jkbkupczyk commented on code in PR #161:
URL: https://github.com/apache/commons-net/pull/161#discussion_r1249330003
##########
src/main/java/org/apache/commons/net/util/Base64.java:
##########
@@ -114,10 +114,10 @@ public class Base64 {
// some state be preserved between calls of encode() and decode().
/**
- * Tests a given byte array to see if it contains only valid characters within the Base64 alphabet.
+ * Tests a given byte array to see if it contains any valid character within the Base64 alphabet.
Review Comment:
thanks 😊
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] garydgregory merged pull request #161: Add Base64 missing tests and documentation fixes
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #161:
URL: https://github.com/apache/commons-net/pull/161
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] kinow commented on a diff in pull request #161: Add Base64 missing tests and documentation fixes
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #161:
URL: https://github.com/apache/commons-net/pull/161#discussion_r1249295029
##########
src/test/java/org/apache/commons/net/util/Base64Test.java:
##########
@@ -208,9 +241,10 @@ public void testEncodeObject() {
}
@Test
- @Ignore
public void testEncodeToString() {
- fail("Not yet implemented");
+ final Base64 base64 = new Base64();
+ final byte[] bytesToEncode = new byte[] {'l', 'i', 'g', 'h', 't', ' ', 'w', 'o', 'r'};
Review Comment:
Random question here; was the example intended to be "light work" encoded? :slightly_smiling_face: Asking as it was used in the other tests, but doesn't really matter as this test is working fine :+1:
##########
src/main/java/org/apache/commons/net/util/Base64.java:
##########
@@ -114,10 +114,10 @@ public class Base64 {
// some state be preserved between calls of encode() and decode().
/**
- * Tests a given byte array to see if it contains only valid characters within the Base64 alphabet.
+ * Tests a given byte array to see if it contains any valid character within the Base64 alphabet.
Review Comment:
Oh, I had to read the code to confirm it. To me the updated text looks correct, great catch!
##########
src/main/java/org/apache/commons/net/util/Base64.java:
##########
@@ -603,10 +603,10 @@ int avail() {
}
/**
- * Decodes a byte[] containing characters in the Base64 alphabet.
+ * Decodes a byte array containing characters in the Base64 alphabet.
*
* @param pArray A byte array containing Base64 character data
- * @return a byte array containing binary data
+ * @return a byte array containing binary data; will return {@code null} if provided byte array is null.
Review Comment:
s/is null/is {@code null}
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] jkbkupczyk commented on a diff in pull request #161: Add Base64 missing tests and documentation fixes
Posted by "jkbkupczyk (via GitHub)" <gi...@apache.org>.
jkbkupczyk commented on code in PR #161:
URL: https://github.com/apache/commons-net/pull/161#discussion_r1249325364
##########
src/test/java/org/apache/commons/net/util/Base64Test.java:
##########
@@ -208,9 +241,10 @@ public void testEncodeObject() {
}
@Test
- @Ignore
public void testEncodeToString() {
- fail("Not yet implemented");
+ final Base64 base64 = new Base64();
+ final byte[] bytesToEncode = new byte[] {'l', 'i', 'g', 'h', 't', ' ', 'w', 'o', 'r'};
Review Comment:
nope, just a random example from [Base64 - Output Padding](https://en.wikipedia.org/wiki/Base64#Output_padding) page in wikipedia 😀
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a diff in pull request #161: Add Base64 missing tests and documentation fixes
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #161:
URL: https://github.com/apache/commons-net/pull/161#discussion_r1249526432
##########
src/test/java/org/apache/commons/net/util/Base64Test.java:
##########
@@ -208,9 +241,10 @@ public void testEncodeObject() {
}
@Test
- @Ignore
public void testEncodeToString() {
- fail("Not yet implemented");
+ final Base64 base64 = new Base64();
+ final byte[] bytesToEncode = new byte[] {'l', 'i', 'g', 'h', 't', ' ', 'w', 'o', 'r'};
Review Comment:
> Random question here; was the example intended to be "light work" encoded? 🙂 Asking as it was used in the other tests, but doesn't really matter as this test is working fine 👍
In the future, we could split up test method per test source, like `testWikipediaExamplesAbc`, `testRfc123FeatureX`, and so on.
--
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@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org