Table of contents

Introduction

Generating random strings is a common task in software development. Whether it’s for generating passwords, unique identifiers, or test data, having a reliable function to generate random strings is essential. However, writing such a function can be more challenging than it seems, as there are many pitfalls that can lead to flawed code.

In this blog post, we will explore the process of refactoring a functionally correct but flawed Go code for generating random strings. We will walk through the steps of identifying the issues in the original code, refactoring it to eliminate those issues, and testing the refactored code to ensure it generates truly random strings. By the end of this post, you will have a better understanding of how to write robust code for generating random strings in Go.

While working on a project, I had the task of creating a random string with a specified length. The simplified version of this task is provided in the following example.

package main

import (
	"crypto/rand"
	"encoding/base64"
	"fmt"
)

func GenerateRandomBytes(size int) []byte {
	randomBytes := make([]byte, size)
	_, err := rand.Read(randomBytes)
	if err != nil {
		return nil
	}

	return randomBytes
}

func GenerateRandomString(length int) string {
	b := GenerateRandomBytes(length)
	return base64.URLEncoding.EncodeToString(b)
}

func main() {
	randomString := GenerateRandomString(12)
	fmt.Println(randomString)
}

The above code is functionally correct but raises few eyebrows when looked into more closely. It generates a slice of random byte based on the size provided and then generates a string based on that.

Issues

The code mentioned above had multiple problems, such as generating random strings of inconsistent length and encoding that was not suitable for use in URLs. This was frustrating, and as any good engineer would, I used the git blame command to see who wrote the problematic code. To my surprise, I discovered that I was the one who wrote it.

a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700  9) func GenerateRandomBytes(size int) ([]byte, error) {
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 10)         randomBytes := make([]byte, size)
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 11)         _, err := rand.Read(randomBytes)
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 12)         if err != nil {
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 13)                 return nil, fmt.Errorf("could not read random bytes: %v", err)
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 14)         }
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 15)         return randomBytes, nil
a69e9325 utils/random.go           (Prabesh Thapa 2022-11-01 17:56:08 -0700 16) }

It’s interesting to find such things and laugh at yourself for writing such shit piece of code.

Ok, back to the topic, as mentioned there were few issues with the code. Here are some of them

First issue

In the GenerateRandomString function, the base64 encoding i.e base64.URLEncoding is used to convert the generated random bytes to a string. However, base64 encoding is not URL safe, so it may lead to issues if the generated string is used in a URL. This can be resolved using base64.RawURLEncoding

Second issue

When generating random bytes, the variable naming convention does not reflect what is actually happening on the code.

	randomBytes := make([]byte, size)

The above code, does not generate randomBytes but instead generates a empty slice of length size with zero value i.e [ 0 0 0 0 0 0 0 0 0 0 0 0 ]

Here the name of variable is misleading which will lead to confusion. Instead, we can avoid this either using Go language suggested format which is writing short var i.e b := make([]byte, size). This way, reader will intrepret code based on what its doing which is initializing a empty slice, and not intrepret code based on its name.

Or we can be a bit descriptive by initializing as newSlice := make([]byte, size). I personally prefer the first one.

Third issue

GenerateRandomString would generate inconsistent length strings. Here the length of the resulting string is not guaranteed to be exactly 12 characters, since the base64 encoding can add padding characters (equals signs) at the end of the string. This can be resolved by stripping out only chars till the length provided using base64.RawURLEncoding.EncodeToString(b)[:length]

Fourth issue

It’s crucial to understand that the base64 encoding method is not inherently secure. Therefore, we must exercise caution and consider the intended use of the generated string. While it may be suitable for creating random strings to represent entities, such as item IDs, we must be extremely careful when using it for sensitive information like passwords, tokens, or nonces. Base64 encoding is not cryptographically secure, so we need to use appropriate encryption methods for such sensitive data.

Conclusion

In conclusion, refactoring functionally correct but flawed code can be a valuable exercise that helps to improve the quality and reliability of your code. After all the things said and done, the refactored would look like this


package main

import (
	"crypto/rand"
	"encoding/base64"
	"fmt"
)

// RandStringBytes generates slice of random bytes. It takes size as an input. The size specifies the length and capacity of the returned byte slice.
func RandStringBytes(size int) ([]byte, error) {

	b := make([]byte, size)

	_, err := rand.Read(b)
	if err != nil {
		return nil, fmt.Errorf("could not read random bytes: %v", err)
	}

	return s, nil
}

// GenerateRandomString generates a random string as per the length provided.
func GenerateRandomString(length int) (string, error) {

	b, err := RandStringBytes(length)
	if err != nil {
		return "", fmt.Errorf("could not generate random bytes: %v", err)
	}

	// Note that the length of the resulting string is not guaranteed to be exactly 12 characters,
	// since the base64 encoding can add padding characters (equals signs) at the end of the string.
  // Hence, we only take chars upto the length from the output	
  
  	s := base64.RawURLEncoding.EncodeToString(b)[:length]
	return s, nil
}

func main() {
  randomString, err := GenerateRandomString(12)
  if err != nil {
    panic("Could not generate random string")
  }
	fmt.Println(randomString)
}

I hope you enjoyed reading this article. Wishing you a good day ahead.