Skip to content

Commit

Permalink
fmt: revert "don't pad strings with zeros"
Browse files Browse the repository at this point in the history
This reverts CL 555776 (commit 704401f).
Scores of tests break inside Google, and there was a test for the old behavior,
so clearly we thought it was correct at one point.

An example of code that broke inside Google is:

	func (pn ProjectNumber) PaddedHexString() string {
		return fmt.Sprintf("%016s", strconv.FormatInt(int64(pn), 16))
	}

Here is another example:

	// IPv4toISO create ISO address base on a given IPv4 address.
	func IPv4toISO(v4 string) (string, error) {
		if net.ParseIP(v4).To4() == nil {
			return "", fmt.Errorf("invalid IPv4 address")
		}
		s := strings.Split(v4, ".")
		var ss string
		for _, n := range s {
			ss = ss + fmt.Sprintf("%03s", n)
		}
		if len(ss) != 12 {
			return "", fmt.Errorf("invalid IPv4 address")
		}
		return fmt.Sprint("49.0001." + ss[0:4] + "." + ss[4:8] + "." + ss[8:12] + ".00"), nil
	}

This is doing the weird but apparently standard conversion from
IPv4 to ISO ISIS Area 1 (see for example [1]).

Here is an example from github.com/netbirdio/netbird:

	func generateNewToken() (string, string, error) {
		secret, err := b.Random(PATSecretLength)
		if err != nil {
			return "", "", err
		}

		checksum := crc32.ChecksumIEEE([]byte(secret))
		encodedChecksum := base62.Encode(checksum)
		paddedChecksum := fmt.Sprintf("%06s", encodedChecksum)
		plainToken := PATPrefix + secret + paddedChecksum
		hashedToken := sha256.Sum256([]byte(plainToken))
		encodedHashedToken := b64.StdEncoding.EncodeToString(hashedToken[:])
		return encodedHashedToken, plainToken, nil
	}

base62.Encode returns a string no leading zeros; the %06s adds leading zeros.

Are there other ways to write these examples? Yes.
Has all this code worked until now? Also yes.

The change to this behavior observed that right padding doesn't
add zeros, only left padding, but that makes sense: in numbers
without decimal points, zeros on the left preserve the value
while zeros on the right change it.

Since we agree that this case is probably not important either way,
preserve the long-time behavior of %0s.

Will document it in a followup CL: this is a clean revert.

Reopen #56486.

[1] https://community.cisco.com/t5/routing/isis-net-address-configuration/m-p/1338984/highlight/true#M127827

Change-Id: Ie7dd35227f46933ccc9bfa1eac5fa8608f6d1918
Reviewed-on: https://go-review.googlesource.com/c/go/+/559196
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
rsc committed Jan 31, 2024
1 parent ae457e8 commit a939bb9
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/fmt/fmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ var fmtTests = []struct {
{"%2s", []byte("\u263a"), " ☺"},
{"%-5s", "abc", "abc "},
{"%-5s", []byte("abc"), "abc "},
{"%05s", "abc", " abc"},
{"%05s", []byte("abc"), " abc"},
{"%05s", "abc", "00abc"},
{"%05s", []byte("abc"), "00abc"},
{"%5s", "abcdefghijklmnopqrstuvwxyz", "abcdefghijklmnopqrstuvwxyz"},
{"%5s", []byte("abcdefghijklmnopqrstuvwxyz"), "abcdefghijklmnopqrstuvwxyz"},
{"%.5s", "abcdefghijklmnopqrstuvwxyz", "abcde"},
Expand Down
5 changes: 0 additions & 5 deletions src/fmt/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,11 +703,6 @@ func (p *pp) printArg(arg any, verb rune) {
return
}

// Bug fix: avoid padding strings with zeros. Issue 56486.
if verb == 's' {
p.fmt.zero = false
}

// Some types can be done without reflection.
switch f := arg.(type) {
case bool:
Expand Down

0 comments on commit a939bb9

Please sign in to comment.