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

proposal: encoding/base64: add AppendEncode and AppendDecode methods to *Encoding #19366

Closed
valyala opened this issue Mar 2, 2017 · 9 comments

Comments

@valyala
Copy link
Contributor

valyala commented Mar 2, 2017

The problem

Currently encoding/base64.Encoding provides not very convenient Encode and Decode methods when working with dst byte slices. These methods require the caller to pre-allocate dst byte slice with the space required for encoding or decoding to succeed. This significantly complicates the resulting code, which should just append base64-encoded (or -decoded) value to the provided dst byte slice. For instance:

// AppendB64Encoded appends base64-encoded src to dst and returns the dst.
func AppendB64(e *base64.Encoding, dst, src []byte) []byte {
    // the caller should make this memory allocation or further complicate the code
    // with byte slice hacks in order to avoid the memory allocation.
    buf := make([]byte, e.EncodedLen(len(src)))
    e.Encode(buf, src)
    return append(dst, buf...)

The solution

It would be great if encoding/base64 would provide AppendEncode and AppendDecode methods on the Encoding with the following signatures:

// AppendEncode appends encoded src to dst and returns the dst (which may be newly allocated).
//
// The encoding pads the output to a multiple of 4 bytes, so AppendEncode is not appropriate for use
// on individual blocks of a large data stream. Use NewEncoder() instead
func (e *Encoding) AppendEncode(dst, src []byte) []byte
// AppendDecode appends decoded src to dst and returns the dst (which may be newly allocated).
//
// If src contains invalid base64 data, it will return CorruptInputError.
// New line characters (\r and \n) are ignored
func (e *Encoding) AppendDecode(dst, src []byte) ([]byte, error)

This would simplify and optimize third-party code that currently uses Encode and Decode methods of Encoding struct.

Standard go packages already contains similar Append* functions - see strconv.Append* for details.

@valyala
Copy link
Contributor Author

valyala commented Mar 2, 2017

Prepared the CL

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/37639 mentions this issue.

@slrz
Copy link

slrz commented Mar 2, 2017

The appending functions in strconv allow callers to avoid the string allocation. That's not necessary with encoding/base64: Encode and Decode already operate on caller-provided buffers.

This change wouldn't allow you to do something you can't do today. It's a minor convenience issue which I don't think warrants additional API surface.

@valyala
Copy link
Contributor Author

valyala commented Mar 3, 2017

The appending functions in strconv allow callers to avoid the string allocation. That's not necessary with encoding/base64: Encode and Decode already operate on caller-provided buffers.

The current API requires at least three additional steps from the caller in order to use Encode or Decode:

// 1. Determine the required dst buffer size:
dstLen := enc.EncodedLen(len(src))

// 2. Allocate the dst buffer
dst := make([]byte, dstLen)

// 3. Encode src into dst
enc.Encode(dst, src)

// 4. Use dst. Usually this means appending dst to another buffer
someBuf = append(someBuf, dst...)

Compare it to the equivalent one-liner if this proposal will be accepted:

someBuf = enc.AppendEncode(someBuf, src)

Additional benefits:

  • someBuf may be nil.
  • it is easy to avoid memory allocations with the new API similar to strconv.Append*.

@bradfitz bradfitz changed the title encoding/base64: add AppendEncode and AppendDecode methods to Encoding proposal: encoding/base64: add AppendEncode and AppendDecode methods to Encoding Mar 21, 2017
@bradfitz
Copy link
Contributor

Upgraded this to a proposal. @golang/proposal-review, there is also some discussion in https://golang.org/cl/37639 but let's keep it here.

@gopherbot gopherbot added this to the Proposal milestone Mar 21, 2017
@rsc
Copy link
Contributor

rsc commented Mar 27, 2017

I think what you really want is:

// AppendB64Encoded appends base64-encoded src to dst and returns the dst.
func AppendB64(e *base64.Encoding, dst, src []byte) []byte {
    // the caller should make this memory allocation or further complicate the code
    // with byte slice hacks in order to avoid the memory allocation.
    old := len(dst)
    need := e.EncodedLen(len(src))
    for cap(dst) - len(dst) < need {
        dst = append(dst[:cap(dst)], 0)
    }
    e.Encode(dst[old:], src)
    return dst[:old+need]
}

Or something like that. It's unclear this needs to be in the standard library.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2017

Based on not much interest and on the fact that one can already write this function without additional help from the standard library, I'm leaning toward declining this proposal. Objections?

@rsc rsc changed the title proposal: encoding/base64: add AppendEncode and AppendDecode methods to Encoding proposal: encoding/base64: add AppendEncode and AppendDecode methods to *Encoding Apr 3, 2017
@rsc
Copy link
Contributor

rsc commented Apr 10, 2017

No objections.

@rsc rsc closed this as completed Apr 10, 2017
@golang golang locked and limited conversation to collaborators Apr 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants