You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "arthurscchan (via GitHub)" <gi...@apache.org> on 2023/11/22 16:56:45 UTC
[PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
arthurscchan opened a new pull request, #222:
URL: https://github.com/apache/commons-codec/pull/222
This fixes a possible IndexOutOfBoundsException in [src/main/java/org/apache/commons/codec/net/PercentCodec.java](https://github.com/apache/commons-codec/blob/master/src/main/java/org/apache/commons/codec/net/PercentCodec.java)
This PR adds a conditional check to ensure only valid bytes (positive or zero) are processed.
We found this bug using fuzzing by way of OSS-Fuzz. It is reported at https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64362.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1826182135
## [Codecov](https://app.codecov.io/gh/apache/commons-codec/pull/222?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`44e4c4d`)](https://app.codecov.io/gh/apache/commons-codec/commit/44e4c4d778c3ab87db09c00e9d1c3260fd42dad5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 92.27% compared to head [(`46991d0`)](https://app.codecov.io/gh/apache/commons-codec/pull/222?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 92.27%.
> Report is 4 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #222 +/- ##
=========================================
Coverage 92.27% 92.27%
- Complexity 1742 1743 +1
=========================================
Files 67 67
Lines 4584 4585 +1
Branches 709 710 +1
=========================================
+ Hits 4230 4231 +1
Misses 242 242
Partials 112 112
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-codec/pull/222?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1824804379
Hi @arthurscchan,
Please help me understand why we need this PR because at first glance this PR could be making the use case worse IMO:
- Before PR: Grabage in -> exception, albeit not `IllegalArgumentException`.
- After PR: Garbage in -> garbage out
Or am I missing something?
TY!
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1825662110
@arthurscchan
Please use a better description in PRs and JIRA: Specify the class and method where the exception occurs.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #222:
URL: https://github.com/apache/commons-codec/pull/222#discussion_r1402656990
##########
src/test/java/org/apache/commons/codec/net/PercentCodecTest.java:
##########
@@ -105,6 +106,13 @@ public void testEncodeUnsupportedObject() {
assertThrows(EncoderException.class, () -> percentCodec.encode("test"));
}
+ @Test
+ public void testInvalidByte() {
+ byte[] invalid = {(byte)-1};
Review Comment:
Use `final` where you can.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory closed pull request #222: CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method
URL: https://github.com/apache/commons-codec/pull/222
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1837588329
Closing: Done.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1824929996
> > Hi, I have added unit test. I am not sure about combining the PRs since they are opened as 4 different issues in the JIRA.
>
> In this case, I don't care about mapping PRs to JIRA 1-1. YMMV ;-) I care about making my life easier ;-)
If you want, I don't mind combining all the PRs together, not sure if it is more messy to handle though.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1824813742
Hi @garydgregory,
In my understanding, the constructor of PecentCodec calls the `insertAlwaysEncodeChars(byte[])` method in [Line 83](https://github.com/apache/commons-codec/blob/master/src/main/java/org/apache/commons/codec/net/PercentCodec.java#L83). The use of the `insertAlwaysEncodeChars(byte[])` method is to record all bytes in the provided byte array to be some character that requires encode all the time. Thus if garbage bytes do exist in the byte array, it should be skipped and all other "valid" bytes should still be considered. For example, if someone thinks that the letter A should always be encoded, and accidentally provides `byte[] encode = {(byte)'A', (byte) -1};`, the logic could still work, skipping the -1 bytes and change the 'A' byte in the BitSet to true. Also, the byte array itself is never stored in the object. Thus the PR could make the running smoother without crashing even if some invalid byte does exist. The new logic simply skips them and continues. The most important part i
s that "valid" bytes are still processed and interpreted.
This is my understanding, am I missing something here??
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1827774135
Hi @garydgregory,
I think it is a different case. The invalid bytes are provided through constructor, it is only used for the initial setting of the PercentCodec object to set which bytes are to be encoded during the encode or decode method. Thus if we initialise the PercentCodec with a byte array contains invalid bytes, it just mean that the encode method will not encode that byte. It does not affect the encode and decode process at all. Thus the encode / decode method round trip won't be affected and the result is the same as the original input. Maybe I could add a round trip call and ensure the result are the same in the test case.
Alternatively, instead of checking the length and skip the invalid byte during constructor phase, I could simply add a try catch block in the code, when any IndexOutOfBound (or more general, and kind of RuntimeException) is thrown, wrap it with EncoderException. This could at least make the method not throwing "unexpected" RuntimeException. It maybe a better approach. WDYT?
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1825686434
> @arthurscchan Please use a better description in PRs and JIRA: Specify the class and method where the exception occurs.
> > > Hi, I have added unit test. I am not sure about combining the PRs since they are opened as 4 different issues in the JIRA.
> >
> >
> > In this case, I don't care about mapping PRs to JIRA 1-1. YMMV ;-) I care about making my life easier ;-)
>
> If you want, I don't mind combining all the PRs together, not sure if it is more messy to handle. Although they are throwing similar exceptions, I would say the reason is not too similar IMO.
Well, at this point, I commented on all PRs, so we might as well leave it as is.
You'll have to make sure to review all comments from all PRs as they apply to all PRs but I did not want to repeat myself all over the place ;-)
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1824505982
> Hi, I have added unit test. I am not sure about combining the PRs since they are opened as 4 different issues in the JIRA.
In this case, I don't care about mapping PRs to JIRA 1-1. YMMV ;-) I care about making my life easier ;-)
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1827764168
@garydgregory Oh, you are right, if we filtered out invalid data, then the round trip result will be differed from the original input. Sorry for missing that. Maybe throwing EncoderException is much better. Instead of checking the length and skip the invalid byte, I could simply add a try catch block in the code, when any IndexOutOfBound (or more general, and kind of RuntimeException) is thrown, wrap it with EncoderException. This could at least make the method not throwing "unexpected" RuntimeException. It maybe a better approach. WDYT?
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #222:
URL: https://github.com/apache/commons-codec/pull/222#discussion_r1409857343
##########
src/main/java/org/apache/commons/codec/net/PercentCodec.java:
##########
@@ -77,10 +77,15 @@ public PercentCodec() {
*
* @param alwaysEncodeChars the unsafe characters that should always be encoded
* @param plusForSpace the flag defining if the space character should be encoded as '+'
+ * @throws EncoderException if the alwaysEncodeChars byte array contains invalid bytes
*/
- public PercentCodec(final byte[] alwaysEncodeChars, final boolean plusForSpace) {
+ public PercentCodec(final byte[] alwaysEncodeChars, final boolean plusForSpace) throws EncoderException {
Review Comment:
Hi @arthurscchan
Now that I look at this again, it doesn't make sense to me, this seems like the exact use case for an `IllegalArgumentException`. In addition, the constructor does not encode anything, so an `EncoderException` really does not make sense IMO.
I propose:
```
diff --git a/src/main/java/org/apache/commons/codec/net/PercentCodec.java b/src/main/java/org/apache/commons/codec/net/PercentCodec.java
index 8e51538..df623df 100644
--- a/src/main/java/org/apache/commons/codec/net/PercentCodec.java
+++ b/src/main/java/org/apache/commons/codec/net/PercentCodec.java
@@ -231,6 +231,9 @@
* @param b the byte that is candidate for min and max limit
*/
private void insertAlwaysEncodeChar(final byte b) {
+ if (b < 0) {
+ throw new IllegalArgumentException("byte must be >= 0");
+ }
this.alwaysEncodeChars.set(b);
if (b < alwaysEncodeCharsMin) {
alwaysEncodeCharsMin = b;
diff --git a/src/test/java/org/apache/commons/codec/net/PercentCodecTest.java b/src/test/java/org/apache/commons/codec/net/PercentCodecTest.java
index 34f8ecb..a537efb 100644
--- a/src/test/java/org/apache/commons/codec/net/PercentCodecTest.java
+++ b/src/test/java/org/apache/commons/codec/net/PercentCodecTest.java
@@ -106,6 +106,12 @@
}
@Test
+ public void testInvalidByte() throws Exception {
+ final byte[] invalid = { (byte) -1, (byte) 'A' };
+ assertThrows(IllegalArgumentException.class, () -> new PercentCodec(invalid, true));
+ }
+
+ @Test
public void testPercentEncoderDecoderWithNullOrEmptyInput() throws Exception {
final PercentCodec percentCodec = new PercentCodec(null, true);
assertNull(percentCodec.encode(null), "Null input value encoding test");
```
WDYT?
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1823363053
Hi, I have added unit test. I am not sure about combining the PRs since they are opened as 4 different issues in the JIRA.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1827848126
Hi @garydgregory, OK. Sorry for not considering in that direction. I will change the code in this PR to wrap around the possible IndexOutOfBoundException in an EncoderException, to at least decrease the change of "unpexected" input thrown directly from the constructor. That maybe a better approach. Thanks for your comments.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1827843610
Hi @arthurscchan
I'm not a fan of making the ctor overly clever in a way that was never documented and is not expected or obvious (IMO). This violates the principle of least surprise (IMO). We then risk some feature-creep, and setting expectations that other constructors should somehow sanitize their inputs. That is not the best approach for a lower-level library like Commons Codec I would offer. I'd rather the ctor throws an exception on garbage input.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on code in PR #222:
URL: https://github.com/apache/commons-codec/pull/222#discussion_r1412032098
##########
src/main/java/org/apache/commons/codec/net/PercentCodec.java:
##########
@@ -77,10 +77,15 @@ public PercentCodec() {
*
* @param alwaysEncodeChars the unsafe characters that should always be encoded
* @param plusForSpace the flag defining if the space character should be encoded as '+'
+ * @throws EncoderException if the alwaysEncodeChars byte array contains invalid bytes
*/
- public PercentCodec(final byte[] alwaysEncodeChars, final boolean plusForSpace) {
+ public PercentCodec(final byte[] alwaysEncodeChars, final boolean plusForSpace) throws EncoderException {
Review Comment:
Yes, I think it is a better changes and interpretation of the exceptions. I will change it accordingly.
@garydgregory
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1828039723
Hi @garydgregory, thanks for your comments and suggestions. I will surely keep that in mind.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1827859750
TY @arthurscchan
Let's keep in mind that this is a library that needs to make sense (or try to) for users. As opposed to just addressing fuzzing issues in the most expedient manner ;-) TY again for your work :-)
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "arthurscchan (via GitHub)" <gi...@apache.org>.
arthurscchan commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1839720505
@garydgregory Sorry I am bit busy and did not handle that last week. Thanks for fixing that for me.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1823156553
Let's just have ONE PR instead of one-liner PRs. Especially since these all claim to fix the exact same kind of issue. Also, you MUST provide a matching test.
--
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
Re: [PR] CODEC-314: Fix possible IndexOutOfBoundsException thrown by PercentCodec.insertAlwaysEncodeChars() method [commons-codec]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #222:
URL: https://github.com/apache/commons-codec/pull/222#issuecomment-1826188320
> Hi @garydgregory,
>
> In my understanding, the constructor of PecentCodec calls the `insertAlwaysEncodeChars(byte[])` method in [Line 83](https://github.com/apache/commons-codec/blob/master/src/main/java/org/apache/commons/codec/net/PercentCodec.java#L83). The use of the `insertAlwaysEncodeChars(byte[])` method is to record all bytes in the provided byte array to be some character that requires encoding. Thus if garbage bytes do exist in the byte array, it should be skipped and all other "valid" bytes should still be considered. For example, if someone thinks that the letter A should always be encoded, and accidentally provides `byte[] encode = {(byte)'A', (byte) -1};`, the logic could still work, skipping the -1 bytes and change the 'A' byte in the BitSet to true. Also, the byte array itself is never stored in the object. Thus the PR could make the running smoother without crashing even if some invalid byte does exist. The new logic simply skips them and continues. The most important part is that "v
alid" bytes are still processed and interpreted. This is my understanding, am I missing something here?? Do let me know if I interpret the functionality wrongly. Thanks.
Hmm, so that means that one can roundtrip encode/decode, but when you start with bad data, you end up with nothing (if all the input is bad) or good encoded data. But when you decode that encoded data, you don't end up with what you started. If we really want all of that, I think we should update the Javadoc and describe that encoding also performs filtering and can never throw an exception for illegal input. WDYT?
Does that mean encode never throws EncoderException?
--
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