Issue with Decoder.bytes: References previous DataView buffer (instead of copying it) about bytes HOT 9 CLOSED

Expanding the offset parameter on this line to bytes.byteOffset + offset (instead of offset only) seems to fix the issue. Both my previous example as my ProtoBuf Decoder run successfully with the change.
I think this is because the DataView.prototype.getUint8 and friends respect the offset of the original DataView, whereas Bytes.Decode.bytes currently ignores it. When using Bytes.Decode.bytes from the start of a Bytes sequence this obviously does not cause any issues.

from bytes.

I just started doing that ;-)

from bytes.

I am also running into this issue with my ProtoBuf decoder. However, I am not sure it has to do with Decode.loop, I think I've seen it with a custom recursion as well.
I tried creating a more basic example where replacing Decode.unsignedInt8 with Decode.bytes 1 causes things to break.

-- 10 bytes
data =
    Hex.toBytes "0102030405060708090A"
        |> Maybe.withDefault (Encode.encode <| Encode.sequence [])

-- 2x5 bytes, i.e. ["0102030405","060708090A"]
list =
    data
        |> Decode.decode
            (Decode.bytes 5
                |> Decode.andThen (\a -> Decode.map (\b -> [ a, b ]) (Decode.bytes 5))
            )
        |> Maybe.withDefault []

-- run the decoder for both items
mapped =
    list
        |> List.map (\bs -> Decode.decode (decoder <| Bytes.width bs) bs)
        |> Debug.log "mapped"

-- the loop decoder
decoder w =
    Decode.loop ( w, [] )
        (\( n, result ) ->
            if n <= 0 then
                Decode.succeed <| Decode.Done result

            else
                --Decode.map (\v -> Decode.Loop ( n - 1, v :: result )) Decode.unsignedInt8
                Decode.map (\v -> Decode.Loop ( n - 1, Hex.fromBytes v :: result )) (Decode.bytes 1)
        )

For the last two Decode.map (\v -> Decode.Loop ... lines, enabling

• Decode.unsignedInt8 correctly results in mapped: [Just [5,4,3,2,1],Just [10,9,8,7,6]]
• Decode.bytes 1 unfortunately results in mapped: [Just ["05","04","03","02","01"],Just ["05","04","03","02","01"]]

Could it have something to do with the fact that Decode.bytes "Copies a given number of bytes into a new Bytes sequence."?

from bytes.

Nice one @eriktim 👌 I think you're right.

The bytes decoder uses this function:
https://github.com/elm/bytes/blob/master/src/Elm/Kernel/Bytes.js#L149
Which uses the buffer from another DataView, and that buffer is a reference.
So if that reference changes, the copy changes as well.
It should be bytes.buffer.slice() if I'm not mistaken (see docs).

Will test out my theory soon when I have some free time.

from bytes.

Yes, I've been looking into that as well. But the referenced data should not change either, should it?
It looks as if the offset gets reset. When splitting the 10 bytes into 6 and 4 (instead of 5-5) you can see the lengths are correct.

from bytes.

@eriktim Awesome 🙌 Do you want to create a PR or should I?

from bytes.

@eriktim Could you tell me how you tested your fix? 🤔 It doesn't let me run the Elm Kernel code.

from bytes.

Sure @icidasset, I tested it by modifying my 1.0.7 installation:

• Apply the patch in ~/.elm/0.19.0/package/elm/bytes/1.0.7/src/Elm/Kernel/Bytes.js;
• Remove all .dat files in ~/.elm/0.19.0/package/elm/bytes/1.0.7;
• Run your code.

Afterwards you can just delete the ~/.elm/0.19.0/package/elm/bytes/1.0.7 directory to restore the changes.

from bytes.

Thanks @eriktim ! I'm going to close this issue in favor of #12, where I explained the issue we have in greater detail and made a smaller SSCCE.

from bytes.