Skip to content

🐛 Probability Distribution Bug in Regex-Based Random String Generation - #6947

Merged
ganigeorgiev merged 1 commit into
pocketbase:developfrom
yerTools:develop
Jun 21, 2025
Merged

🐛 Probability Distribution Bug in Regex-Based Random String Generation#6947
ganigeorgiev merged 1 commit into
pocketbase:developfrom
yerTools:develop

Conversation

@yerTools

Copy link
Copy Markdown
Contributor

Thank you so much for this project. ❤️

📋 Issue Summary

I've discovered a probability distribution bug in PocketBase's regex-based random string generation system that affects the randomness and uniqueness of generated identifiers.

🔍 Problem Description

When using character classes in regex patterns for ID generation, the current implementation creates a severe bias toward smaller character ranges. This is particularly problematic when using URL-safe base64 patterns like [A-Za-z0-9\-_]{30}, where single characters (- and _) appear far more frequently than they should.

The issue also affects the default pattern [a-z0-9]{15}, creating uneven distribution between letters and numbers.

🎯 Examples of the Problem

Here are some examples of generated strings using [A-Za-z0-9\-_]{30} showing the excessive occurrence of - and _:

Vw39d_h776XXabv-NJ_---_2-_Y__J
--i_6yLb_3-io54_8NP-5Vp_I-JZb6
i_UoCw-2---W-__w35_3RvS__-5-M-
--sLJdC-_lH-xD_9--9fx-_Xb4_-m-
-M15_qe_R3C_fVp7M--_-715-mDUeo
f-b-MImP_xhg4Y-O--_WrdGK-_-_2s
lnoE--_yN-m0_b2_L7timLg_FzaRhX
_S-S_v-q4Z-H____q0_-i-____I__V
-_6-___2PvL_-K__tNg_T__B-Oqg50
1O7NH__r2G_aI-9-1--_ok--__v-62

Notice: The strings are heavily biased toward - and _ characters, creating poor randomness distribution.

After this fix the generated strings look like this:

AJAW5tt737M-tDCdVjjpQMfiwlvD9B
F5mGcR4xuGU38EjFb-L5GGxCP0iPkd
E7zoxVWKAGf29agVoXQdUNY2VH_CLw
Ue8SfAfM9ASJ7KPR3y6PQoGcxky2Y_
8hnPS6nqv7tAhl_fpsgoHiXTbGKq1F
VCt9AW2YdlHA294ee8MRI0ALw64UEQ
AOLraN8t6UvyB6acRzHavlDXpYi3aZ
XnYr3Euap_6llHaeEnQXnlTqudQkZn
jCsYcKn1fepprFlwFzdwInvP5t36rs
KfAaZcDEwq_E-jDat3Eu__cV85Z9Ul

⚡ Cause

The current implementation in tools/security/random_by_regex.go processes character classes incorrectly:

func randomRuneFromPairs(pairs []rune) (rune, error) {
    idx, err := randomNumber(len(pairs) / 2)
    if err != nil {
        return 0, err
    }
    return randomRuneFromRange(pairs[idx*2], pairs[idx*2+1])
}

Splitting into Ranges

For the pattern [A-Za-z0-9\-_]{30}, the regex compiler creates these range pairs:

Range Characters Count Current Probability Expected Probability
A-Z A,B,C...Z 26 20% ~40.6%
a-z a,b,c...z 26 20% ~40.6%
0-9 0,1,2...9 10 20% ~15.6%
- - 1 20% ~1.6%
_ _ 1 20% ~1.6%

The Problem

Current approach: Randomly selects a range first (each gets 20%), then picks a character from that range
Better approach: Each individual character should have equal probability (~1.56% for 64 total characters)

Result: Single characters get 12.8x higher probability than they should!

Additional Context

The input pairs array for the base64 pattern becomes:
['-', '-', '0', '9', 'A', 'Z', '_', '_', 'a', 'z']

Even when writing the regex explicitly as:
[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789\-_]{30}
The compiler still optimizes it to ranges, resulting in the same biased behavior.


🛠️ Proposed Solution

I've developed a corrected implementation that uses weighted random selection to ensure uniform character distribution:

func randomRuneFromPairs(pairs []rune) (rune, error) {
	if len(pairs)%2 != 0 {
		return 0, fmt.Errorf("invalid pairs slice: odd number of elements")
	}

	// Pre-calculate the cumulative size of all ranges to make the selection process more efficient.
	cumulativeSizes := make([]int, len(pairs)/2)
	totalRunes := 0
	for i := 0; i < len(pairs); i += 2 {
		start, end := pairs[i], pairs[i+1]
		if start > end {
			return 0, fmt.Errorf("invalid range: start '%c' > end '%c'", start, end)
		}
		totalRunes += int(end - start + 1)
		cumulativeSizes[i/2] = totalRunes
	}

	if totalRunes == 0 {
		return 0, errors.New("no runes to choose from")
	}

	// Select a random number in the range of total runes.
	runeNumber, err := randomNumber(totalRunes)
	if err != nil {
		return 0, fmt.Errorf("failed to generate random number: %w", err)
	}

	// Find which range the selected number falls into using the pre-calculated cumulative sizes.
	for i, size := range cumulativeSizes {
		if runeNumber < size {
			startRune := pairs[i*2]
			previousSize := 0
			if i > 0 {
				previousSize = cumulativeSizes[i-1]
			}
			return startRune + rune(runeNumber-previousSize), nil
		}
	}

	// This part should be unreachable if the logic is correct.
	// It indicates a bug in this function or in randomNumber.
	panic("unreachable: failed to find a rune")
}

🎯 This assures a correct probability distribution: Each character gets equal probability.


📊 Performance Benchmarks

I conducted tests and benchmarks to validate both correctness and performance improvements:

Benchmark Results

Implementation Pattern Iterations Time/op Memory/op Allocs/op
Current (Buggy) [a-z0-9]{15} 361,990 3,236 ns 1,840 B 97
Fixed [a-z0-9]{15} 477,039 2,433 ns 1,360 B 67
Current (Buggy) [A-Za-z0-9\-_]{30} 187,546 6,181 ns 3,312 B 178
Fixed [A-Za-z0-9\-_]{30} 271,593 4,237 ns 3,408 B 130

🧪 Statistical Validation

I ran extensive statistical tests with 500,000 iterations to verify the probability distribution fixes:

🎯 Key Results Summary

Pattern Implementation Standard Deviation Status
[a-z0-9]{15} Current (Buggy) 1.40% ❌ Biased
[a-z0-9]{15} Fixed 0.01% ✅ Uniform
[A-Za-z0-9\-_]{30} Current (Buggy) 3.37% ❌ Severely Biased
[A-Za-z0-9\-_]{30} Fixed 0.00% ✅ Perfect Distribution

Detailed Test Results

Lower Alpha-Numerical Pattern: [a-z0-9]{15} (500k iterations)

Current Implementation standard deviation: 1.40%
  • 0: Count: 373935, Percentage: 4.99%
  • 1: Count: 375151, Percentage: 5.00%
  • 2: Count: 375199, Percentage: 5.00%
  • 3: Count: 375345, Percentage: 5.00%
  • 4: Count: 375677, Percentage: 5.01%
  • 5: Count: 375172, Percentage: 5.00%
  • 6: Count: 375182, Percentage: 5.00%
  • 7: Count: 376248, Percentage: 5.02%
  • 8: Count: 374385, Percentage: 4.99%
  • 9: Count: 375316, Percentage: 5.00%
  • a: Count: 144097, Percentage: 1.92%
  • b: Count: 144382, Percentage: 1.93%
  • c: Count: 143071, Percentage: 1.91%
  • d: Count: 144743, Percentage: 1.93%
  • e: Count: 143769, Percentage: 1.92%
  • f: Count: 144354, Percentage: 1.92%
  • g: Count: 144086, Percentage: 1.92%
  • h: Count: 144624, Percentage: 1.93%
  • i: Count: 144393, Percentage: 1.93%
  • j: Count: 144134, Percentage: 1.92%
  • k: Count: 144344, Percentage: 1.92%
  • l: Count: 143677, Percentage: 1.92%
  • m: Count: 143942, Percentage: 1.92%
  • n: Count: 144387, Percentage: 1.93%
  • o: Count: 144265, Percentage: 1.92%
  • p: Count: 143811, Percentage: 1.92%
  • q: Count: 144057, Percentage: 1.92%
  • r: Count: 144318, Percentage: 1.92%
  • s: Count: 144086, Percentage: 1.92%
  • t: Count: 143582, Percentage: 1.91%
  • u: Count: 144125, Percentage: 1.92%
  • v: Count: 144619, Percentage: 1.93%
  • w: Count: 144542, Percentage: 1.93%
  • x: Count: 144634, Percentage: 1.93%
  • y: Count: 144549, Percentage: 1.93%
  • z: Count: 143799, Percentage: 1.92%
Fixed Implementation standard deviation: 0.01%
  • 0: Count: 207817, Percentage: 2.77%
  • 1: Count: 208467, Percentage: 2.78%
  • 2: Count: 208336, Percentage: 2.78%
  • 3: Count: 207442, Percentage: 2.77%
  • 4: Count: 207867, Percentage: 2.77%
  • 5: Count: 207851, Percentage: 2.77%
  • 6: Count: 208950, Percentage: 2.79%
  • 7: Count: 208687, Percentage: 2.78%
  • 8: Count: 208469, Percentage: 2.78%
  • 9: Count: 208209, Percentage: 2.78%
  • a: Count: 208805, Percentage: 2.78%
  • b: Count: 207595, Percentage: 2.77%
  • c: Count: 208346, Percentage: 2.78%
  • d: Count: 208771, Percentage: 2.78%
  • e: Count: 208408, Percentage: 2.78%
  • f: Count: 208171, Percentage: 2.78%
  • g: Count: 208835, Percentage: 2.78%
  • h: Count: 208600, Percentage: 2.78%
  • i: Count: 208525, Percentage: 2.78%
  • j: Count: 207299, Percentage: 2.76%
  • k: Count: 208349, Percentage: 2.78%
  • l: Count: 208224, Percentage: 2.78%
  • m: Count: 208648, Percentage: 2.78%
  • n: Count: 208462, Percentage: 2.78%
  • o: Count: 207665, Percentage: 2.77%
  • p: Count: 208658, Percentage: 2.78%
  • q: Count: 208817, Percentage: 2.78%
  • r: Count: 208350, Percentage: 2.78%
  • s: Count: 208447, Percentage: 2.78%
  • t: Count: 208835, Percentage: 2.78%
  • u: Count: 208583, Percentage: 2.78%
  • v: Count: 208496, Percentage: 2.78%
  • w: Count: 209050, Percentage: 2.79%
  • x: Count: 207390, Percentage: 2.77%
  • y: Count: 208524, Percentage: 2.78%
  • z: Count: 208052, Percentage: 2.77%

Alpha-Numerical Pattern: [A-Za-z0-9]{15} (500k iterations)

Current Implementation standard deviation: 0.76%
  • 0: Count: 250800, Percentage: 3.34%
  • 1: Count: 249670, Percentage: 3.33%
  • 2: Count: 249506, Percentage: 3.33%
  • 3: Count: 250264, Percentage: 3.34%
  • 4: Count: 250538, Percentage: 3.34%
  • 5: Count: 250189, Percentage: 3.34%
  • 6: Count: 249382, Percentage: 3.33%
  • 7: Count: 249545, Percentage: 3.33%
  • 8: Count: 250473, Percentage: 3.34%
  • 9: Count: 250856, Percentage: 3.34%
  • A: Count: 96462, Percentage: 1.29%
  • B: Count: 96542, Percentage: 1.29%
  • C: Count: 96366, Percentage: 1.28%
  • D: Count: 95718, Percentage: 1.28%
  • E: Count: 95807, Percentage: 1.28%
  • F: Count: 96264, Percentage: 1.28%
  • G: Count: 96948, Percentage: 1.29%
  • H: Count: 95795, Percentage: 1.28%
  • I: Count: 96333, Percentage: 1.28%
  • J: Count: 96121, Percentage: 1.28%
  • K: Count: 95776, Percentage: 1.28%
  • L: Count: 95860, Percentage: 1.28%
  • M: Count: 96221, Percentage: 1.28%
  • N: Count: 96520, Percentage: 1.29%
  • O: Count: 96238, Percentage: 1.28%
  • P: Count: 96060, Percentage: 1.28%
  • Q: Count: 96221, Percentage: 1.28%
  • R: Count: 96354, Percentage: 1.28%
  • S: Count: 96226, Percentage: 1.28%
  • T: Count: 95666, Percentage: 1.28%
  • U: Count: 95872, Percentage: 1.28%
  • V: Count: 95690, Percentage: 1.28%
  • W: Count: 95574, Percentage: 1.27%
  • X: Count: 96255, Percentage: 1.28%
  • Y: Count: 96027, Percentage: 1.28%
  • Z: Count: 96518, Percentage: 1.29%
  • a: Count: 97085, Percentage: 1.29%
  • b: Count: 97006, Percentage: 1.29%
  • c: Count: 96269, Percentage: 1.28%
  • d: Count: 96465, Percentage: 1.29%
  • e: Count: 96326, Percentage: 1.28%
  • f: Count: 96513, Percentage: 1.29%
  • g: Count: 96040, Percentage: 1.28%
  • h: Count: 96369, Percentage: 1.28%
  • i: Count: 95803, Percentage: 1.28%
  • j: Count: 95803, Percentage: 1.28%
  • k: Count: 95580, Percentage: 1.27%
  • l: Count: 95695, Percentage: 1.28%
  • m: Count: 96105, Percentage: 1.28%
  • n: Count: 95586, Percentage: 1.27%
  • o: Count: 96450, Percentage: 1.29%
  • p: Count: 96160, Percentage: 1.28%
  • q: Count: 96170, Percentage: 1.28%
  • r: Count: 95735, Percentage: 1.28%
  • s: Count: 95711, Percentage: 1.28%
  • t: Count: 95781, Percentage: 1.28%
  • u: Count: 95751, Percentage: 1.28%
  • v: Count: 96108, Percentage: 1.28%
  • w: Count: 96668, Percentage: 1.29%
  • x: Count: 95721, Percentage: 1.28%
  • y: Count: 96190, Percentage: 1.28%
  • z: Count: 96253, Percentage: 1.28%
Fixed implementation standard deviation: 0.00%
  • 0: Count: 121027, Percentage: 1.61%
  • 1: Count: 121161, Percentage: 1.62%
  • 2: Count: 120685, Percentage: 1.61%
  • 3: Count: 120983, Percentage: 1.61%
  • 4: Count: 120443, Percentage: 1.61%
  • 5: Count: 121489, Percentage: 1.62%
  • 6: Count: 120828, Percentage: 1.61%
  • 7: Count: 120678, Percentage: 1.61%
  • 8: Count: 120849, Percentage: 1.61%
  • 9: Count: 121047, Percentage: 1.61%
  • A: Count: 120948, Percentage: 1.61%
  • B: Count: 121242, Percentage: 1.62%
  • C: Count: 120510, Percentage: 1.61%
  • D: Count: 120392, Percentage: 1.61%
  • E: Count: 120842, Percentage: 1.61%
  • F: Count: 121073, Percentage: 1.61%
  • G: Count: 120771, Percentage: 1.61%
  • H: Count: 121041, Percentage: 1.61%
  • I: Count: 120502, Percentage: 1.61%
  • J: Count: 121002, Percentage: 1.61%
  • K: Count: 121703, Percentage: 1.62%
  • L: Count: 121166, Percentage: 1.62%
  • M: Count: 120842, Percentage: 1.61%
  • N: Count: 120937, Percentage: 1.61%
  • O: Count: 121151, Percentage: 1.62%
  • P: Count: 121720, Percentage: 1.62%
  • Q: Count: 121288, Percentage: 1.62%
  • R: Count: 121054, Percentage: 1.61%
  • S: Count: 120891, Percentage: 1.61%
  • T: Count: 120285, Percentage: 1.60%
  • U: Count: 120922, Percentage: 1.61%
  • V: Count: 121077, Percentage: 1.61%
  • W: Count: 120977, Percentage: 1.61%
  • X: Count: 120842, Percentage: 1.61%
  • Y: Count: 121429, Percentage: 1.62%
  • Z: Count: 120356, Percentage: 1.60%
  • a: Count: 121471, Percentage: 1.62%
  • b: Count: 121273, Percentage: 1.62%
  • c: Count: 120997, Percentage: 1.61%
  • d: Count: 121624, Percentage: 1.62%
  • e: Count: 121058, Percentage: 1.61%
  • f: Count: 120703, Percentage: 1.61%
  • g: Count: 120633, Percentage: 1.61%
  • h: Count: 120686, Percentage: 1.61%
  • i: Count: 120924, Percentage: 1.61%
  • j: Count: 120675, Percentage: 1.61%
  • k: Count: 120682, Percentage: 1.61%
  • l: Count: 120708, Percentage: 1.61%
  • m: Count: 120933, Percentage: 1.61%
  • n: Count: 121381, Percentage: 1.62%
  • o: Count: 121289, Percentage: 1.62%
  • p: Count: 121003, Percentage: 1.61%
  • q: Count: 121037, Percentage: 1.61%
  • r: Count: 120738, Percentage: 1.61%
  • s: Count: 121067, Percentage: 1.61%
  • t: Count: 121454, Percentage: 1.62%
  • u: Count: 121116, Percentage: 1.61%
  • v: Count: 121057, Percentage: 1.61%
  • w: Count: 120582, Percentage: 1.61%
  • x: Count: 120776, Percentage: 1.61%
  • y: Count: 120843, Percentage: 1.61%
  • z: Count: 121137, Percentage: 1.62%

Alphabetical Pattern: [A-Za-z]{15} (500k iterations)

Current Implementation standard deviation: 0.01%
  • A: Count: 144695, Percentage: 1.93%
  • B: Count: 144030, Percentage: 1.92%
  • C: Count: 144560, Percentage: 1.93%
  • D: Count: 144576, Percentage: 1.93%
  • E: Count: 144199, Percentage: 1.92%
  • F: Count: 144718, Percentage: 1.93%
  • G: Count: 144377, Percentage: 1.93%
  • H: Count: 144006, Percentage: 1.92%
  • I: Count: 144809, Percentage: 1.93%
  • J: Count: 144749, Percentage: 1.93%
  • K: Count: 144114, Percentage: 1.92%
  • L: Count: 143668, Percentage: 1.92%
  • M: Count: 144593, Percentage: 1.93%
  • N: Count: 144020, Percentage: 1.92%
  • O: Count: 144532, Percentage: 1.93%
  • P: Count: 144271, Percentage: 1.92%
  • Q: Count: 144359, Percentage: 1.92%
  • R: Count: 144024, Percentage: 1.92%
  • S: Count: 144434, Percentage: 1.93%
  • T: Count: 144253, Percentage: 1.92%
  • U: Count: 144454, Percentage: 1.93%
  • V: Count: 143844, Percentage: 1.92%
  • W: Count: 144178, Percentage: 1.92%
  • X: Count: 143611, Percentage: 1.91%
  • Y: Count: 145117, Percentage: 1.93%
  • Z: Count: 144671, Percentage: 1.93%
  • a: Count: 143368, Percentage: 1.91%
  • b: Count: 144101, Percentage: 1.92%
  • c: Count: 143935, Percentage: 1.92%
  • d: Count: 143822, Percentage: 1.92%
  • e: Count: 143896, Percentage: 1.92%
  • f: Count: 144278, Percentage: 1.92%
  • g: Count: 143964, Percentage: 1.92%
  • h: Count: 143997, Percentage: 1.92%
  • i: Count: 144308, Percentage: 1.92%
  • j: Count: 143973, Percentage: 1.92%
  • k: Count: 144274, Percentage: 1.92%
  • l: Count: 143926, Percentage: 1.92%
  • m: Count: 144137, Percentage: 1.92%
  • n: Count: 144251, Percentage: 1.92%
  • o: Count: 145050, Percentage: 1.93%
  • p: Count: 144199, Percentage: 1.92%
  • q: Count: 144545, Percentage: 1.93%
  • r: Count: 144659, Percentage: 1.93%
  • s: Count: 143799, Percentage: 1.92%
  • t: Count: 144169, Percentage: 1.92%
  • u: Count: 143897, Percentage: 1.92%
  • v: Count: 143322, Percentage: 1.91%
  • w: Count: 144485, Percentage: 1.93%
  • x: Count: 144276, Percentage: 1.92%
  • y: Count: 144464, Percentage: 1.93%
  • z: Count: 144043, Percentage: 1.92%
Fixed implementation standard deviation: 0.00%
  • A: Count: 143610, Percentage: 1.91%
  • B: Count: 144294, Percentage: 1.92%
  • C: Count: 144220, Percentage: 1.92%
  • D: Count: 144104, Percentage: 1.92%
  • E: Count: 143579, Percentage: 1.91%
  • F: Count: 144273, Percentage: 1.92%
  • G: Count: 144645, Percentage: 1.93%
  • H: Count: 143674, Percentage: 1.92%
  • I: Count: 145014, Percentage: 1.93%
  • J: Count: 143832, Percentage: 1.92%
  • K: Count: 144136, Percentage: 1.92%
  • L: Count: 144391, Percentage: 1.93%
  • M: Count: 144045, Percentage: 1.92%
  • N: Count: 144434, Percentage: 1.93%
  • O: Count: 144567, Percentage: 1.93%
  • P: Count: 143749, Percentage: 1.92%
  • Q: Count: 144310, Percentage: 1.92%
  • R: Count: 144527, Percentage: 1.93%
  • S: Count: 144148, Percentage: 1.92%
  • T: Count: 144304, Percentage: 1.92%
  • U: Count: 144688, Percentage: 1.93%
  • V: Count: 144368, Percentage: 1.92%
  • W: Count: 143685, Percentage: 1.92%
  • X: Count: 144036, Percentage: 1.92%
  • Y: Count: 143993, Percentage: 1.92%
  • Z: Count: 144601, Percentage: 1.93%
  • a: Count: 144385, Percentage: 1.93%
  • b: Count: 144710, Percentage: 1.93%
  • c: Count: 144690, Percentage: 1.93%
  • d: Count: 144621, Percentage: 1.93%
  • e: Count: 144666, Percentage: 1.93%
  • f: Count: 144058, Percentage: 1.92%
  • g: Count: 144455, Percentage: 1.93%
  • h: Count: 143737, Percentage: 1.92%
  • i: Count: 144033, Percentage: 1.92%
  • j: Count: 144028, Percentage: 1.92%
  • k: Count: 143897, Percentage: 1.92%
  • l: Count: 144746, Percentage: 1.93%
  • m: Count: 143902, Percentage: 1.92%
  • n: Count: 144264, Percentage: 1.92%
  • o: Count: 144079, Percentage: 1.92%
  • p: Count: 143746, Percentage: 1.92%
  • q: Count: 143955, Percentage: 1.92%
  • r: Count: 144321, Percentage: 1.92%
  • s: Count: 144092, Percentage: 1.92%
  • t: Count: 144343, Percentage: 1.92%
  • u: Count: 144606, Percentage: 1.93%
  • v: Count: 144434, Percentage: 1.93%
  • w: Count: 144350, Percentage: 1.92%
  • x: Count: 144374, Percentage: 1.92%
  • y: Count: 144363, Percentage: 1.92%
  • z: Count: 143918, Percentage: 1.92%

Numerical Pattern: [0-9]{15} (500k iterations)

Current Implementation standard deviation: 0.01%
  • 0: Count: 749682, Percentage: 10.00%
  • 1: Count: 749813, Percentage: 10.00%
  • 2: Count: 751089, Percentage: 10.01%
  • 3: Count: 749724, Percentage: 10.00%
  • 4: Count: 750357, Percentage: 10.00%
  • 5: Count: 750371, Percentage: 10.00%
  • 6: Count: 750832, Percentage: 10.01%
  • 7: Count: 749724, Percentage: 10.00%
  • 8: Count: 748786, Percentage: 9.98%
  • 9: Count: 749622, Percentage: 9.99%
Fixed implementation standard deviation: 0.00%
  • 0: Count: 749246, Percentage: 9.99%
  • 1: Count: 749157, Percentage: 9.99%
  • 2: Count: 749503, Percentage: 9.99%
  • 3: Count: 750682, Percentage: 10.01%
  • 4: Count: 750731, Percentage: 10.01%
  • 5: Count: 751192, Percentage: 10.02%
  • 6: Count: 749690, Percentage: 10.00%
  • 7: Count: 749966, Percentage: 10.00%
  • 8: Count: 750003, Percentage: 10.00%
  • 9: Count: 749830, Percentage: 10.00%

Base64 Pattern: [A-Za-z0-9\-_]{30} (500k iterations)

Current Implementation standard deviation: 3.37% - Notice the extreme bias toward - and _
  • -: Count: 2997786, Percentage: 19.99%
  • 0: Count: 299994, Percentage: 2.00%
  • 1: Count: 299642, Percentage: 2.00%
  • 2: Count: 300325, Percentage: 2.00%
  • 3: Count: 300798, Percentage: 2.01%
  • 4: Count: 300013, Percentage: 2.00%
  • 5: Count: 299990, Percentage: 2.00%
  • 6: Count: 299492, Percentage: 2.00%
  • 7: Count: 298374, Percentage: 1.99%
  • 8: Count: 299702, Percentage: 2.00%
  • 9: Count: 300926, Percentage: 2.01%
  • A: Count: 115250, Percentage: 0.77%
  • B: Count: 114854, Percentage: 0.77%
  • C: Count: 115648, Percentage: 0.77%
  • D: Count: 115362, Percentage: 0.77%
  • E: Count: 115612, Percentage: 0.77%
  • F: Count: 115445, Percentage: 0.77%
  • G: Count: 115359, Percentage: 0.77%
  • H: Count: 115447, Percentage: 0.77%
  • I: Count: 115320, Percentage: 0.77%
  • J: Count: 115376, Percentage: 0.77%
  • K: Count: 115134, Percentage: 0.77%
  • L: Count: 115382, Percentage: 0.77%
  • M: Count: 115367, Percentage: 0.77%
  • N: Count: 114858, Percentage: 0.77%
  • O: Count: 115726, Percentage: 0.77%
  • P: Count: 115564, Percentage: 0.77%
  • Q: Count: 115294, Percentage: 0.77%
  • R: Count: 115633, Percentage: 0.77%
  • S: Count: 115776, Percentage: 0.77%
  • T: Count: 115321, Percentage: 0.77%
  • U: Count: 115580, Percentage: 0.77%
  • V: Count: 115572, Percentage: 0.77%
  • W: Count: 115369, Percentage: 0.77%
  • X: Count: 114622, Percentage: 0.76%
  • Y: Count: 115773, Percentage: 0.77%
  • Z: Count: 115981, Percentage: 0.77%
  • _: Count: 2999348, Percentage: 20.00%
  • a: Count: 115487, Percentage: 0.77%
  • b: Count: 115382, Percentage: 0.77%
  • c: Count: 115544, Percentage: 0.77%
  • d: Count: 115507, Percentage: 0.77%
  • e: Count: 115451, Percentage: 0.77%
  • f: Count: 115717, Percentage: 0.77%
  • g: Count: 115612, Percentage: 0.77%
  • h: Count: 115958, Percentage: 0.77%
  • i: Count: 115256, Percentage: 0.77%
  • j: Count: 115305, Percentage: 0.77%
  • k: Count: 115148, Percentage: 0.77%
  • l: Count: 114893, Percentage: 0.77%
  • m: Count: 115522, Percentage: 0.77%
  • n: Count: 115148, Percentage: 0.77%
  • o: Count: 115488, Percentage: 0.77%
  • p: Count: 115106, Percentage: 0.77%
  • q: Count: 115683, Percentage: 0.77%
  • r: Count: 116429, Percentage: 0.78%
  • s: Count: 115767, Percentage: 0.77%
  • t: Count: 115034, Percentage: 0.77%
  • u: Count: 115937, Percentage: 0.77%
  • v: Count: 115499, Percentage: 0.77%
  • w: Count: 115716, Percentage: 0.77%
  • x: Count: 115306, Percentage: 0.77%
  • y: Count: 115957, Percentage: 0.77%
  • z: Count: 115133, Percentage: 0.77%
Fixed Implementation standard deviation: 0.00% - All characters now have equal ~1.56% probability!
  • -: Count: 235201, Percentage: 1.57%
  • 0: Count: 234780, Percentage: 1.57%
  • 1: Count: 234164, Percentage: 1.56%
  • 2: Count: 234541, Percentage: 1.56%
  • 3: Count: 234729, Percentage: 1.56%
  • 4: Count: 234657, Percentage: 1.56%
  • 5: Count: 234069, Percentage: 1.56%
  • 6: Count: 234711, Percentage: 1.56%
  • 7: Count: 234021, Percentage: 1.56%
  • 8: Count: 234731, Percentage: 1.56%
  • 9: Count: 234463, Percentage: 1.56%
  • A: Count: 234316, Percentage: 1.56%
  • B: Count: 235700, Percentage: 1.57%
  • C: Count: 234133, Percentage: 1.56%
  • D: Count: 233918, Percentage: 1.56%
  • E: Count: 234297, Percentage: 1.56%
  • F: Count: 233793, Percentage: 1.56%
  • G: Count: 234857, Percentage: 1.57%
  • H: Count: 234550, Percentage: 1.56%
  • I: Count: 234543, Percentage: 1.56%
  • J: Count: 234545, Percentage: 1.56%
  • K: Count: 234356, Percentage: 1.56%
  • L: Count: 234911, Percentage: 1.57%
  • M: Count: 235906, Percentage: 1.57%
  • N: Count: 234571, Percentage: 1.56%
  • O: Count: 234815, Percentage: 1.57%
  • P: Count: 233921, Percentage: 1.56%
  • Q: Count: 234152, Percentage: 1.56%
  • R: Count: 234395, Percentage: 1.56%
  • S: Count: 234728, Percentage: 1.56%
  • T: Count: 233209, Percentage: 1.55%
  • U: Count: 234493, Percentage: 1.56%
  • V: Count: 234851, Percentage: 1.57%
  • W: Count: 235070, Percentage: 1.57%
  • X: Count: 234845, Percentage: 1.57%
  • Y: Count: 233643, Percentage: 1.56%
  • Z: Count: 234275, Percentage: 1.56%
  • _: Count: 234856, Percentage: 1.57%
  • a: Count: 233728, Percentage: 1.56%
  • b: Count: 234661, Percentage: 1.56%
  • c: Count: 233751, Percentage: 1.56%
  • d: Count: 234070, Percentage: 1.56%
  • e: Count: 233904, Percentage: 1.56%
  • f: Count: 234413, Percentage: 1.56%
  • g: Count: 234203, Percentage: 1.56%
  • h: Count: 234269, Percentage: 1.56%
  • i: Count: 234820, Percentage: 1.57%
  • j: Count: 233859, Percentage: 1.56%
  • k: Count: 234413, Percentage: 1.56%
  • l: Count: 234526, Percentage: 1.56%
  • m: Count: 234582, Percentage: 1.56%
  • n: Count: 233725, Percentage: 1.56%
  • o: Count: 233850, Percentage: 1.56%
  • p: Count: 233324, Percentage: 1.56%
  • q: Count: 233822, Percentage: 1.56%
  • r: Count: 234361, Percentage: 1.56%
  • s: Count: 234882, Percentage: 1.57%
  • t: Count: 234053, Percentage: 1.56%
  • u: Count: 233262, Percentage: 1.56%
  • v: Count: 234577, Percentage: 1.56%
  • w: Count: 234696, Percentage: 1.56%
  • x: Count: 233742, Percentage: 1.56%
  • y: Count: 234118, Percentage: 1.56%
  • z: Count: 234673, Percentage: 1.56%

🎯 Impact & Security Implications

Current Impact

  • 🔒 Security Risk: Predictable patterns in generated IDs reduce entropy
  • 📊 Poor Randomness: Biased character distribution affects cryptographic strength
  • 🔍 Pattern Recognition: Attackers could potentially exploit the bias

🖹 My test and benchmark implementation

Here is my test file I used to write this fix.
You can simply copy-paste it into an empty go file and debug it or run the benchmarks/tests for yourself. 😄

issue_test.go
package main

import (
	cryptoRand "crypto/rand"
	"errors"
	"fmt"
	"math"
	"math/big"
	"regexp/syntax"
	"slices"
	"strings"
	"testing"
)

const defaultMaxRepeat = 6

var anyCharNotNLPairs = []rune{'A', 'Z', 'a', 'z', '0', '9'}

func RandomStringByRegex_old(pattern string, optFlags ...syntax.Flags) (string, error) {
	var flags syntax.Flags
	if len(optFlags) == 0 {
		flags = syntax.Perl
	} else {
		for _, f := range optFlags {
			flags |= f
		}
	}

	r, err := syntax.Parse(pattern, flags)
	if err != nil {
		return "", err
	}

	var sb = new(strings.Builder)

	err = writeRandomStringByRegex_old(r, sb)
	if err != nil {
		return "", err
	}

	return sb.String(), nil
}

func writeRandomStringByRegex_old(r *syntax.Regexp, sb *strings.Builder) error {
	// https://pkg.go.dev/regexp/syntax#Op
	switch r.Op {
	case syntax.OpCharClass:
		c, err := randomRuneFromPairs_old(r.Rune)
		if err != nil {
			return err
		}
		_, err = sb.WriteRune(c)
		return err
	case syntax.OpAnyChar, syntax.OpAnyCharNotNL:
		c, err := randomRuneFromPairs_old(anyCharNotNLPairs)
		if err != nil {
			return err
		}
		_, err = sb.WriteRune(c)
		return err
	case syntax.OpAlternate:
		idx, err := randomNumber(len(r.Sub))
		if err != nil {
			return err
		}
		return writeRandomStringByRegex_old(r.Sub[idx], sb)
	case syntax.OpConcat:
		var err error
		for _, sub := range r.Sub {
			err = writeRandomStringByRegex_old(sub, sb)
			if err != nil {
				break
			}
		}
		return err
	case syntax.OpRepeat:
		return repeatRandomStringByRegex_old(r.Sub[0], sb, r.Min, r.Max)
	case syntax.OpQuest:
		return repeatRandomStringByRegex_old(r.Sub[0], sb, 0, 1)
	case syntax.OpPlus:
		return repeatRandomStringByRegex_old(r.Sub[0], sb, 1, -1)
	case syntax.OpStar:
		return repeatRandomStringByRegex_old(r.Sub[0], sb, 0, -1)
	case syntax.OpCapture:
		return writeRandomStringByRegex_old(r.Sub[0], sb)
	case syntax.OpLiteral:
		_, err := sb.WriteString(string(r.Rune))
		return err
	default:
		return fmt.Errorf("unsupported pattern operator %d", r.Op)
	}
}

func repeatRandomStringByRegex_old(r *syntax.Regexp, sb *strings.Builder, min int, max int) error {
	if max < 0 {
		max = defaultMaxRepeat
	}

	if max < min {
		max = min
	}

	n := min
	if max != min {
		randRange, err := randomNumber(max - min)
		if err != nil {
			return err
		}
		n += randRange
	}

	var err error
	for i := 0; i < n; i++ {
		err = writeRandomStringByRegex_old(r, sb)
		if err != nil {
			return err
		}
	}

	return nil
}

func randomRuneFromPairs_old(pairs []rune) (rune, error) {
	idx, err := randomNumber(len(pairs) / 2)
	if err != nil {
		return 0, err
	}

	return randomRuneFromRange(pairs[idx*2], pairs[idx*2+1])
}

func randomRuneFromRange(min rune, max rune) (rune, error) {
	offset, err := randomNumber(int(max - min + 1))
	if err != nil {
		return min, err
	}

	return min + rune(offset), nil
}

func randomNumber(maxSoft int) (int, error) {
	randRange, err := cryptoRand.Int(cryptoRand.Reader, big.NewInt(int64(maxSoft)))

	return int(randRange.Int64()), err
}

func RandomStringByRegex_new(pattern string, optFlags ...syntax.Flags) (string, error) {
	var flags syntax.Flags
	if len(optFlags) == 0 {
		flags = syntax.Perl
	} else {
		for _, f := range optFlags {
			flags |= f
		}
	}

	r, err := syntax.Parse(pattern, flags)
	if err != nil {
		return "", err
	}

	var sb = new(strings.Builder)

	err = writeRandomStringByRegex_new(r, sb)
	if err != nil {
		return "", err
	}

	return sb.String(), nil
}

func writeRandomStringByRegex_new(r *syntax.Regexp, sb *strings.Builder) error {
	// https://pkg.go.dev/regexp/syntax#Op
	switch r.Op {
	case syntax.OpCharClass:
		c, err := randomRuneFromPairs_new(r.Rune)
		if err != nil {
			return err
		}
		_, err = sb.WriteRune(c)
		return err
	case syntax.OpAnyChar, syntax.OpAnyCharNotNL:
		c, err := randomRuneFromPairs_new(anyCharNotNLPairs)
		if err != nil {
			return err
		}
		_, err = sb.WriteRune(c)
		return err
	case syntax.OpAlternate:
		idx, err := randomNumber(len(r.Sub))
		if err != nil {
			return err
		}
		return writeRandomStringByRegex_new(r.Sub[idx], sb)
	case syntax.OpConcat:
		var err error
		for _, sub := range r.Sub {
			err = writeRandomStringByRegex_new(sub, sb)
			if err != nil {
				break
			}
		}
		return err
	case syntax.OpRepeat:
		return repeatRandomStringByRegex_new(r.Sub[0], sb, r.Min, r.Max)
	case syntax.OpQuest:
		return repeatRandomStringByRegex_new(r.Sub[0], sb, 0, 1)
	case syntax.OpPlus:
		return repeatRandomStringByRegex_new(r.Sub[0], sb, 1, -1)
	case syntax.OpStar:
		return repeatRandomStringByRegex_new(r.Sub[0], sb, 0, -1)
	case syntax.OpCapture:
		return writeRandomStringByRegex_new(r.Sub[0], sb)
	case syntax.OpLiteral:
		_, err := sb.WriteString(string(r.Rune))
		return err
	default:
		return fmt.Errorf("unsupported pattern operator %d", r.Op)
	}
}

func repeatRandomStringByRegex_new(r *syntax.Regexp, sb *strings.Builder, min int, max int) error {
	if max < 0 {
		max = defaultMaxRepeat
	}

	if max < min {
		max = min
	}

	n := min
	if max != min {
		randRange, err := randomNumber(max - min)
		if err != nil {
			return err
		}
		n += randRange
	}

	var err error
	for i := 0; i < n; i++ {
		err = writeRandomStringByRegex_new(r, sb)
		if err != nil {
			return err
		}
	}

	return nil
}

func randomRuneFromPairs_new(pairs []rune) (rune, error) {
	if len(pairs)%2 != 0 {
		return 0, fmt.Errorf("invalid pairs slice: odd number of elements")
	}

	// Pre-calculate the cumulative size of all ranges to make the selection process more efficient.
	cumulativeSizes := make([]int, len(pairs)/2)
	totalRunes := 0
	for i := 0; i < len(pairs); i += 2 {
		start, end := pairs[i], pairs[i+1]
		if start > end {
			return 0, fmt.Errorf("invalid range: start '%c' > end '%c'", start, end)
		}
		totalRunes += int(end - start + 1)
		cumulativeSizes[i/2] = totalRunes
	}

	if totalRunes == 0 {
		return 0, errors.New("no runes to choose from")
	}

	// Select a random number in the range of total runes.
	runeNumber, err := randomNumber(totalRunes)
	if err != nil {
		return 0, fmt.Errorf("failed to generate random number: %w", err)
	}

	// Find which range the selected number falls into using the pre-calculated cumulative sizes.
	for i, size := range cumulativeSizes {
		if runeNumber < size {
			startRune := pairs[i*2]
			previousSize := 0
			if i > 0 {
				previousSize = cumulativeSizes[i-1]
			}
			return startRune + rune(runeNumber-previousSize), nil
		}
	}

	// This part should be unreachable if the logic is correct.
	// It indicates a bug in this function or in randomNumber.
	panic("unreachable: failed to find a rune")
}

func BenchmarkOld_LowerAlphaNumerical(b *testing.B) {
	for b.Loop() {
		RandomStringByRegex_old("[a-z0-9]{15}")
	}
}

func BenchmarkNew_LowerAlphaNumerical(b *testing.B) {
	for b.Loop() {
		RandomStringByRegex_new("[a-z0-9]{15}")
	}
}

func BenchmarkOld_Base64(b *testing.B) {
	for b.Loop() {
		RandomStringByRegex_old("[A-Za-z0-9\\-_]{30}")
	}
}

func BenchmarkNew_Base64(b *testing.B) {
	for b.Loop() {
		RandomStringByRegex_new("[A-Za-z0-9\\-_]{30}")
	}
}

func fillMap(start rune, end rune) func(map[rune]int) {
	return func(m map[rune]int) {
		for r := start; r <= end; r++ {
			m[r] = 0
		}
	}
}

func testMap(ranges ...func(map[rune]int)) map[rune]int {
	m := make(map[rune]int)

	for _, filler := range ranges {
		filler(m)
	}
	return m
}

func checkMap(t *testing.T, m map[rune]int) {
	total := 0
	for _, count := range m {
		total += count
	}

	meanPercentage := 100.0 / float64(len(m))
	squaredDeviationSum := 0.0

	runes := make([]rune, 0, len(m))
	for k := range m {
		runes = append(runes, k)
	}

	slices.Sort(runes)

	for _, r := range runes {
		count := m[r]

		deviation := 100.0*float64(count)/float64(total) - meanPercentage
		squaredDeviation := deviation * deviation
		squaredDeviationSum += squaredDeviation

		t.Logf("Rune: '%c', Count: %d, Percentage: %.2f%%", r, count, float64(count)/float64(total)*100)
	}

	variance := squaredDeviationSum / float64(len(m)-1)
	standardDeviation := math.Sqrt(variance)

	var output func(format string, args ...any)
	if standardDeviation < 0.05 {
		output = t.Logf
	} else {
		output = t.Fatalf
	}

	output("Standard Deviation: %.2f%", standardDeviation)
}

func TestOld_LowerAlphaNumerical(t *testing.T) {
	m := testMap(
		fillMap('a', 'z'),
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_old("[a-z0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestOld__AlphaNumerical(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_old("[A-Za-z0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestOld_Alphabetical(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_old("[A-Za-z]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestOld_Numerical(t *testing.T) {
	m := testMap(
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_old("[0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestOld_Base64(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
		fillMap('0', '9'),
		fillMap('-', '-'),
		fillMap('_', '_'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_old("[A-Za-z0-9\\-_]{30}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestNew_LowerAlphaNumerical(t *testing.T) {
	m := testMap(
		fillMap('a', 'z'),
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_new("[a-z0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestNew__AlphaNumerical(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_new("[A-Za-z0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestNew_Alphabetical(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_new("[A-Za-z]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestNew_Numerical(t *testing.T) {
	m := testMap(
		fillMap('0', '9'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_new("[0-9]{15}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

func TestNew_Base64(t *testing.T) {
	m := testMap(
		fillMap('A', 'Z'),
		fillMap('a', 'z'),
		fillMap('0', '9'),
		fillMap('-', '-'),
		fillMap('_', '_'),
	)

	for range 500_000 {
		val, err := RandomStringByRegex_new("[A-Za-z0-9\\-_]{30}")
		if err != nil {
			t.Fatalf("failed to generate random string: %v", err)
		}
		for _, r := range val {
			m[r]++
		}
	}

	checkMap(t, m)
}

@ganigeorgiev

ganigeorgiev commented Jun 19, 2025

Copy link
Copy Markdown
Member

There is way too much information in this PR (and I hope it is not AI generated) to quickly understand what is the issue and what the suggested changes do BUT I'll be able to have a more detailed look at it later this weekend.

I'll just address the "Security Risk impact" you've mentioned - keep in mind that security.RandomStringByRegex is not intended (there is even a comment regarding this) for use in critical secure contexts and security.RandomString should be used instead as it generates a cryptographically random string. The identifiers of the records are NOT considered a security sensitive information. In any case, if you have concerns regarding security it would have been better to be first discussed at support@pocketbase.io before publicly open an issue or PR for it.

@yerTools

yerTools commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

Sorry, I didn't mean to be rude. 😅

I probably shouldn't have written a wall of text since the change is more or less trivial.

I don't think it's that relevant for security, but if a user creates a column in the admin interface with an autogenerated pattern, they probably expect more randomness than the current implementation, and this PR should fix it. Since you never know how certain features are (mis-)used.

Anyway, sorry for this huge PR description. I hope the code changes speak for themselves then, and thank you very much not only for your answer but also for this amazing project! ❤️

@ganigeorgiev

Copy link
Copy Markdown
Member

I was able to read through the PR description and I think I understand now what is the issue you are trying to solve.

I appreciate the tests and I'll run them tomorrow on a clear head, but at quick glance over the suggested implementation it feels a little error prone and difficult to read. Wouldn't it be easier to:

  1. fill a slice with all runes from the pairs
  2. generate a random number within the resulting length
  3. return allRunes[random]

Or is performance the reason that you chose the current implementation?

@yerTools

Copy link
Copy Markdown
Contributor Author

TBH I just tried to stick to the original code as much as possible and didn't think much about performance. 😇

I think you have a good point regarding readability.
If you calculate the total number of runes to reduce allocations and GC pressure, most of the code will be the same.


You can take a look at the code for allRunes (with pre-allocation)
func randomRuneFromPairs(pairs []rune) (rune, error) {
	if len(pairs)%2 != 0 {
		return 0, fmt.Errorf("invalid pairs slice: odd number of elements")
	}

	// Pre-calculate the total number of runes in all ranges.
	totalRunes := 0
	for i := 0; i < len(pairs); i += 2 {
		start, end := pairs[i], pairs[i+1]
		if start > end {
			return 0, fmt.Errorf("invalid range: start '%c' > end '%c'", start, end)
		}
		totalRunes += int(end - start + 1)
	}

	if totalRunes == 0 {
		return 0, errors.New("no runes to choose from")
	}

	// Create a slice to hold all runes in the ranges.
	allRunes := make([]rune, 0, totalRunes)
	for i := 0; i < len(pairs); i += 2 {
		start, end := pairs[i], pairs[i+1]
		for r := start; r <= end; r++ {
			allRunes = append(allRunes, r)
		}
	}

	// Select a random number in the range of total runes.
	runeNumber, err := randomNumber(totalRunes)
	if err != nil {
		return 0, fmt.Errorf("failed to generate random number: %w", err)
	}

	return allRunes[runeNumber], nil
}

You can take a look at the code for allRunes (without pre-allocation)
func randomRuneFromPairs(pairs []rune) (rune, error) {
	if len(pairs)%2 != 0 {
		return 0, fmt.Errorf("invalid pairs slice: odd number of elements")
	}

	// Create a slice to hold all runes in the ranges.
	allRunes := make([]rune, 0)
	for i := 0; i < len(pairs); i += 2 {
		start, end := pairs[i], pairs[i+1]
		for r := start; r <= end; r++ {
			allRunes = append(allRunes, r)
		}
	}

	if len(allRunes) == 0 {
		return 0, errors.New("no runes to choose from")
	}

	// Select a random number in the range of total runes.
	runeNumber, err := randomNumber(len(allRunes))
	if err != nil {
		return 0, fmt.Errorf("failed to generate random number: %w", err)
	}

	return allRunes[runeNumber], nil
}

The benchmark with pre-allocation produced similar results to the implementation before this PR. But without pre-allocation the garbage collector has to work a lot more.
Implementation Pattern Iterations Time/op Memory/op Allocs/op
Current (Buggy) [a-z0-9]{15} 369,079 3,395 ns 1,840 B 97
Fixed (Previous) [a-z0-9]{15} 489,627 2,477 ns 1,360 B 67
Fixed (allRunes pre-allocated) [a-z0-9]{15} 344,450 3,206 ns 3,280 B 67
Fixed (allRunes no pre-allocation) [a-z0-9]{15} 217,016 5,368 ns 8,680 B 142
--- --- --- --- --- ---
Current (Buggy) [A-Za-z0-9]{15} 366,669 3,533 ns 1,872 B 98
Fixed (Previous) [A-Za-z0-9]{15} 515,947 2,230 ns 1,512 B 68
Fixed (allRunes pre-allocated) [A-Za-z0-9]{15} 372,850 3,429 ns 4,992 B 68
Fixed (allRunes no pre-allocation) [A-Za-z0-9]{15} 222,608 5,315 ns 8,712 B 143
--- --- --- --- --- ---
Current (Buggy) [A-Za-z0-9\-_]{30} 180,786 6,559 ns 3,312 B 178
Fixed (Previous) [A-Za-z0-9\-_]{30} 273,751 4,314 ns 3,408 B 130
Fixed (allRunes pre-allocated) [A-Za-z0-9\-_]{30} 175,682 6,647 ns 9,648 B 130
Fixed (allRunes no pre-allocation) [A-Za-z0-9\-_]{30} 123,751 9,999 ns 17,088 B 280

Personal opinion - IMHO:

I think Time/op is not very important in this case.

As far as I'm concerned, this method is only used for auto-generated row values.
This means (IMHO) the following:

  • Even with 10 µs per execution (on my cheap laptop with an AMD Ryzen 5 8640HS), this means there could be about 100,000 executions per second, and this call can also be run concurrently. I think in this case, when you have 100k SQLite inserts per second, this shouldn't be a bottleneck.

I think that readability and maintainability are way more important than performance, and there also isn't much room for performance improvements (at least in this PR).


If you like another implementation more than the one currently suggested by this PR, I can update it. 😄

Anyway, thank you very much! ❤️

@ganigeorgiev

Copy link
Copy Markdown
Member

Thank you for trying it and rerunning the tests. I've also run them locally and the above reported numbers seems accurate.

I find the allRunes version more readable but considering that this is used only here and probably will never have to change, there is no need to bikeshed further so let's merge the PR as it is for now.

I'll try to prepare a minor release sometime later this weekend.

@ganigeorgiev
ganigeorgiev merged commit 1610729 into pocketbase:develop Jun 21, 2025
warpbuild-benchmark-bot Bot added a commit to WarpBuilds/pocketbase that referenced this pull request Jun 21, 2025
@A1X5H04

A1X5H04 commented Aug 18, 2025

Copy link
Copy Markdown

Never seen a PR so well written, awesome work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants