Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corner-case bug in Reed Solomon code? #997

Open
nasamaher opened this issue Apr 23, 2018 · 4 comments
Open

Corner-case bug in Reed Solomon code? #997

nasamaher opened this issue Apr 23, 2018 · 4 comments
Labels

Comments

@nasamaher
Copy link

Hi,

First, thanks for this great library. I'm just using the Reed Solomon part of it and I stumbled on I think a corner-case bug.

When using data+error_codes of size 256, if I corrupt the first byte (after ReedSolomon encoding), neither the byte is "repaired" nor an exception is thrown when decoding. It doesn't seem to matter how many error_codes I use or the data content, but it only fails for data+error_codes of size 256.

I grep'd the ReedSolomon code for literals 255 and 256 but saw nothing fishy.

Hmm, code formatter on this wiki not working for me. JUnit test below. Depends on RSEncoderDecoder class, which looks fine by inspection (attached as '.txt')
RSEncoderDecoder.txt

import java.util.Arrays;
import org.junit.Test;
import com.casualcoding.reedsolomon.RSEncoderDecoder;
public class ReedSolomonByte0Bug
{
@test
public void testRSStreamAllZeros() throws Exception
{
/*
* numEcBytes doesn't seem to matter
*/
int numEcBytes = 8;

    for (int totalBlockSizeBytes = 10; totalBlockSizeBytes <= 256; totalBlockSizeBytes++)
    {

        byte[] mesg = new byte[totalBlockSizeBytes - numEcBytes];

        for (int i = 0; i < mesg.length; i++)
        {
            mesg[i] = (byte) i;
        }

        RSEncoderDecoder rs = new RSEncoderDecoder();
        byte[] rsmesg = rs.encodeData(mesg, numEcBytes);

        /*
         * Corrupt byte 0.  Corrupting other bytes works correctly
         */
        rsmesg[0] = (byte) (rsmesg[0] + 1);

        // No exception thrown ...
        byte[] testDecode = rs.decodeData(rsmesg, numEcBytes);

        /*
         * Fails for totalBlockSize = 256
         */
        if (Arrays.equals(mesg, testDecode) == false)
        {
            System.out.println("Failed: totalBlockSizeBytes = " + totalBlockSizeBytes);
        }
    }
}

}

@srowen
Copy link
Contributor

srowen commented Apr 24, 2018

Here's a more direct version of your test:

    int numEcBytes = 8;
    for (int totalBlockSizeBytes = 9; totalBlockSizeBytes <= 256; totalBlockSizeBytes++) {
      int[] mesg = new int[totalBlockSizeBytes];
      for (int i = 0; i < mesg.length - numEcBytes; i++) {
        mesg[i] = i;
      }

      new ReedSolomonEncoder(GenericGF.QR_CODE_FIELD_256).encode(mesg, numEcBytes);
      mesg[0] += 1;
      new ReedSolomonDecoder(GenericGF.QR_CODE_FIELD_256).decode(mesg, numEcBytes);

      for (int i = 0; i < mesg.length - numEcBytes; i++) {
        Assert.assertEquals("totalBlockSizeBytes, i = " + totalBlockSizeBytes + ", " + i, i, mesg[i]);
      }
    }

I don't think this a bug though. You're using a field with 256 elements (I presume) and so the error locator can't express more than 256 distinct error locations. Your input has 257 elements though.

Try using a different field like AZTEC_DATA_10 and it will all work.

@srowen srowen closed this as completed Apr 24, 2018
@srowen
Copy link
Contributor

srowen commented Apr 24, 2018

Hm, not quite; the failure occurs at 256 elements, not 257. I suspect my answer is still roughly correct, but the subtlety of why it works only at 255 elements is beyond me in looking at this for 15 minutes. It may still be a corner case bug, yeah, but maybe narrower than it seems.

@srowen srowen reopened this Apr 24, 2018
@srowen srowen added the bug label Jun 13, 2018
@zxing zxing deleted a comment from nguyenhoaithuong8989 Jul 25, 2020
@zxing zxing deleted a comment from nguyenhoaithuong8989 Jul 25, 2020
@Gary19751957

This comment was marked as off-topic.

@Ayyan551

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

7 participants
@srowen @nasamaher @Gary19751957 @Ayyan551 and others