encoding/ascii85: Decode is wrong when the source is 1 or 5 bytes long

[xformerfhs]

Go version

go version go1.25.1 windows/amd64

Output of go env in your module/workspace:

set AR=ar
set CC=gcc
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_ENABLED=0
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set CXX=g++
set GCCGO=gccgo
set GO111MODULE=
set GOAMD64=v3
set GOARCH=amd64
set GOAUTH=netrc
set GOBIN=
set GOCACHE=C:\Users\User\AppData\Local\go-build
set GOCACHEPROG=
set GODEBUG=
set GOENV=C:\Users\User\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFIPS140=off
set GOFLAGS=
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=T:\UserTemp\User\go-build2668626540=/tmp/go-build -gno-record-gcc-switches
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMOD=D:\Users\User\go\src\cryptochallenge\go.mod
set GOMODCACHE=D:\Users\User\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\Users\User\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTELEMETRY=on
set GOTELEMETRYDIR=C:\Users\User\AppData\Roaming\go\telemetry
set GOTMPDIR=T:\UserTemp\User
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.25.1
set GOWORK=
set PKG_CONFIG=pkg-config

What did you do?

I wrote a program that had to encode data in various encodings. The data are byte slices with variable lengths. One of the encodings is ascii85. I used the standard package encoding/ascii85 for this. I noticed that the decoding was wrong in certain cases and wrote test cases for these:

package byteencoding

import (
	"encoding/ascii85"
	"testing"
)

func TestAscii85EncodeDecode1(t *testing.T) {
	src := []byte("Q")
	enc := make([]byte, ascii85.MaxEncodedLen(len(src)))
	nenc := ascii85.Encode(enc, src)
	enc = enc[0:nenc]
	dec := make([]byte, len(enc))
	ndec, _, err := ascii85.Decode(dec, enc, true)
	if err != nil {
		t.Fatal("Decode failed", err)
	}
	dec = dec[0:ndec]
	if ndec != len(src) {
		t.Fatalf("Decode(%q=Encode(%02x))=%02x returned wrong length. Expected %d, got %d",
			string(enc), src, dec, len(src), ndec)
	}
}

func TestAscii85EncodeDecode5(t *testing.T) {
	src := []byte("ABCDE")
	enc := make([]byte, ascii85.MaxEncodedLen(len(src)))
	nenc := ascii85.Encode(enc, src)
	enc = enc[0:nenc]
	dec := make([]byte, len(enc))
	ndec, _, err := ascii85.Decode(dec, enc, true)
	if err != nil {
		t.Fatal("Decode failed", err)
	}
	dec = dec[0:ndec]
	if ndec != len(src) {
		t.Fatalf("Decode(%q=Encode(%02x))=%02x returned wrong length. Expected %d, got %d",
			string(enc), src, dec, len(src), ndec)
	}
}

I think the culprit is this code right at the beginning of the Decode function:

	if len(dst)-ndst < 4 {
			return
		}

When I comment this out, everything works correctly.

What did you see happen?

When the data to be encoded has size 1 or 5 the encoded value is wrongly decoded. It is one byte short. E.g.

• []byte(0x41, 0x42, 0x43, 0x44, 0x45) correctly encoded to "5sdq,70", incorrectly decoded to []byte(0x41, 0x42, 0x43, 0x44).
• []byte(0x51) correctly encoded to ";#", incorrectly decoded to []byte().

This happens with all data of length 1 and 5.

What did you expect to see?

The data should be correctly decoded. E.g.

• "5sdq,70" should be decoded to []byte(0x41, 0x42, 0x43, 0x44, 0x45).
• ";#", should be decoded to []byte(0x51).

[gabyhelp]

Related Issues

• encoding/base32: uses more space in buffer than returned in EncodedLen method #53606 (closed)
• encoding/base32: Decoder with NoPadding doesn't fill the passed buffer #65166 (closed)
• encoding/hex: panic on hex.Decode with not enough space of dst #34982 (closed)
• encoding/base64: imprecise error for the base64-decoding input with an invalid length #38045
• cmd/compile: checking len(data) and erroring out if too small does not help BCE #19126 (closed)

Related Discussions

• [go-nuts] Different decoding than original source
• [go-nuts] Possible blug on Ascii85
• encoding/base64 : Looks like an off-by-one error in decodedLen()
• [go-nuts] Ascii85 - length at decoding
• base32 DecodeLen() miscalculation when no padding

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

I agree it's a bug, and that that's the right fix. The decoder is being a little too conservative when checking for available space in the destination buffer. Would you like to send a CL to fix the bug and add this test case? Thanks.

[xformerfhs]

Hi, @adonovan

thanks for your answer. I can do what you requested if there is a documentation on what you expect, i.e. how should the change list be prepared, where to send it to, etc. Is there a documentation you can point me to?

While I thought about this it occurred to me that there is no clear definition of how the Encode and Decode functions should behave when the dst slice is short. Right now they simply panic with an error message like panic: runtime error: index out of range [1] with length 1. Is that how it is supposed to be?

I found this bug with the following test function:

import (
	"bytes"
	crand "crypto/rand"
	"encoding/ascii85"
	"math/rand/v2"
	"testing"
)

func TestRandomEncodeDecode(t *testing.T) {
	const supLen = 100
	srcBuf := make([]byte, supLen)
	encBuf := make([]byte, ascii85.MaxEncodedLen(supLen))
	decBuf := make([]byte, supLen)
	for i := 0; i < 1_000; i++ {
		srcLen := rand.IntN(supLen) + 1
		src := srcBuf[:srcLen]
		_, _ = crand.Read(src)
		encDst := encBuf[:ascii85.MaxEncodedLen(srcLen)]
		nenc := ascii85.Encode(encDst, src)
		encDst = encDst[0:nenc]

		decDst := decBuf[0:srcLen]
		ndec, _, err := ascii85.Decode(decDst, encDst, true)
		if err != nil {
			t.Fatal("Decode failed", err)
		}
		decDst = decDst[0:ndec]
		if !bytes.Equal(src, decDst) {
			t.Fatalf("Decode(%q)=%02x, instead of %02x", encDst, decDst, src)
		}
	}
}

Maybe such a test would be useful, as well.

[ianlancetaylor]

The contribution guidelines are at https://go.dev/doc/contribute. Thanks!

Yes, please include a test with any change. The test should fail before the change and pass after it.

It's fine for both Encode and Decode to panic if the destination slice is too short. People who want more flexibility can use NewEncoder and NewDecoder.

[gopherbot]

Change https://go.dev/cl/709315 mentions this issue: encoding/ascii85: let Decode work with short inputs

[xformerfhs]

I just read the comment of @seankhliao on the mentioned change and I do not understand it.

1. The comment of the Decode function does not mention that the length of the destination array has to be a multiple of 4.
2. This version of the ascii85 encoding does not require that the length of the source data has to be multiple of 4.
3. I removed the lines that check the remaining length in the destination buffer and it works perfectly. I use this in production. So I do not understand where the sentence "I don't really see how to make the existing code work with just the exact output dst size." comes from. It does work.
4. A destination slice that has the exact same size of the original data size is in my opinion definitely not short. The data fits exactly. There is no shortness. A round-trip of encoding and decoding should reproduce the original data with the original length and should not require a larger length.