Regex parser breaks if it hits a newline

[jyn514]

In 5f390a0, added newline to encoding comment, breaking the regex, as referenced in 9f9459d. This should not happen.

[charlesdaniels]

I moved re.sub("|".join(string.whitespace), "", p) before the re.findall a few commits ago... I forget exactly why, I believe it was related to trying to get the regex to work in some edge case... you might try moving it back before running re.findall.

I believe the commit responsible is a27fac8

[charlesdaniels]

Has this bug been fixed? If not, can @jyn514 provide a representative example demonstrating that it is not fixed (ideally in the form of a test case)?

[jyn514]

This is not fixed. The regex parser will break if the message is

((<<This is encoded with BitShuffle,
 which you can download from https://github.com/charlesdaniels/bitshuffle|1|base64|bz2|0|0|11e10247b630a3b7a7935d2faabbfb0a6d8183fa|QlpoNTFBWSZTWTs44u4AAALRgAAQQAACYBQAIAAxDAEGT0LwgiUXckU4UJA7OOLu|279d9035886d4c0427549863c4c2101e4a63e041>>))

with the message
Nothing to decode or nothing matched spec. Aborting.

Working on test case now. For a quick demonstration in python:

>>> import re
>>> msg = '''((<<This is encoded with BitShuffle,     
... which you can download from https://github.com/charlesdaniels/bitshuffle|1|base64|bz2|0|0|11e10247b630a3b7a7935d2faabbfb0a6d8183fa|QlpoNTFBWSZTWTs44u4AAALRgAAQQAACYBQAIAAxDAEGT0LwgiUXckU4UJA7OOLu|279d9035886d4c0427549863c4c2101e4a63e041>>))
... '''
>>> packets = re.findall('\(\(<<(.*)>>\)\)', msg,
...                                  flags=re.MULTILINE)
>>> packets
[]

[jyn514]

A possible solution could be to strip white space before seperating packets, instead of after. This would be relatively simple to implement.

[jyn514]

Figured it out - flag should be re.DOTALL, not re.MULTILINE. Adding fix now.

[charlesdaniels]

Is that not how it’s implemented already? This should work, and in fact I thoughts that’s already what we were doing!

…

On Jan 1, 2018, at 5:59 PM, Joshua Nelson ***@***.***> wrote: A possible solution could be to strip white space before seperating packets, instead of after. This would be relatively simple to implement. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[jyn514]

@charlesdaniels currently we're stripping whitespace before processing segments, but not before seperating packets. The reason I'm loath to strip whitespace from packets is because if we do have multi-line comments in the future, they will appear to be only a single line.

[jyn514]

You know what, disregard that, the comment should be thrown away in any case.

[charlesdaniels]

You know what, disregard that, the comment should be thrown away in any case.

Actually, that’s a good test case , especially if we let the user override the comment messsge. What happens if the comment is a valid BitShuffle packet? The correct behavior would be to drop it and do nothing with the inner packet, although that might prove to be difficult as we would presumably be discarding the comment after the regex has already been applied... in any case, the comment should definitely be discarded entirely while decoding - we don’t care if we mangle the line endings in it since the decoder doesn’t even care if the comment is empty.

[jyn514]

I'm getting very strange errors trying to fix this, not really sure where to go from here.

Diff

joshua@debian-thinkpad:~/src/bitshuffle$ git diff
diff --git bitshuffle/bitshuffle.py bitshuffle/bitshuffle.py
index 706d655..aeb7574 100755
--- bitshuffle/bitshuffle.py
+++ bitshuffle/bitshuffle.py
@@ -301,6 +301,10 @@ def decode(message):
        comment, compatibility, encoding, compression, seq_num, \
            seq_end, checksum, chunk = range(8)

+        # delete unused whitespace and separators
+        message = re.sub("|".join(string.whitespace), "", message)
+        stderr.write(message)
+
        try:
            packets = re.findall('\(\(<<(.*)>>\)\)', message,
                                flags=re.MULTILINE)
@@ -312,10 +316,6 @@ def decode(message):
            print("Nothing to decode or nothing matched spec. Aborting.")
            sys.exit(2)

-        # delete unused whitespace and separators
-        packets = [re.sub("|".join(string.whitespace), "", p)
-                   for p in packets if p.strip() is not '']
-
        segments = [None] * len(packets)  # ordered by index of packets
        # each chunk will be appended and original will be returned
        payload = bytes()

Traceback

File "/home/joshua/Documents/src/bitshuffle/scripts/../bitshuffle/bitshuffle.py", line 439, in <module>
    main()
    File "/home/joshua/Documents/src/bitshuffle/scripts/../bitshuffle/bitshuffle.py", line 251, in main
    payload, checksum_ok = decode(args.input.read())
    File "/home/joshua/Documents/src/bitshuffle/scripts/../bitshuffle/bitshuffle.py", line 339, in decode
    payload = bz2.decompress(payload)
    ValueError: couldn't find end of stream

This happens no matter what I change from the original:

• Stripping whitespace before decoding packets
• Using re.DOTALL instead of re.MULTILINE (how would this possibly break anything ?!?)
• Doing both at once

I tried printing the message to stderr and it looks pretty normal. Could use some advice.

[charlesdaniels]

It could be a padding issue. bz2 has very little tolerance for errors in streams. I am slightly dubious of the re.sub you added, it dosen’t feel quite right.

Possibly useful would be the string normalization from lexitar. It’s not exactly identical, but the method for collapsing white space seems like it might be worth a shot in this case (just replace with ’’ instead of ’ ‘).