Skip to content

refactor: use timestamp+filename for bulk message request ID #903

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.

Sign up for GitHub

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

Merged
AchoArnold merged 3 commits into from
May 24, 2026
Merged

refactor: use timestamp+filename for bulk message request ID #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b534091
refactor: use timestamp+filename for bulk message request ID
AchoArnold May 24, 2026
7b29eb5
fix: address PR review comments
AchoArnold May 24, 2026
ada4b90
fix: allow space character in sanitizeFilename
AchoArnold May 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion api/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ require (
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible
github.com/jszwec/csvutil v1.10.0
github.com/lib/pq v1.12.3
github.com/matoous/go-nanoid/v2 v2.1.0
github.com/nyaruka/phonenumbers v1.7.2
github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177
github.com/patrickmn/go-cache v2.1.0+incompatible
Expand Down
2 changes: 0 additions & 2 deletions api/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/lib/pq v1.12.3 h1:tTWxr2YLKwIvK90ZXEw8GP7UFHtcbTtty8zsI+YjrfQ=
github.com/lib/pq v1.12.3/go.mod h1:/p+8NSbOcwzAEI7wiMXFlgydTwcgTr3OSKMsD2BitpA=
github.com/matoous/go-nanoid/v2 v2.1.0 h1:P64+dmq21hhWdtvZfEAofnvJULaRR1Yib0+PnU669bE=
github.com/matoous/go-nanoid/v2 v2.1.0/go.mod h1:KlbGNQ+FhrUNIHUxZdL63t7tl4LaPkZNpUULS8H4uVM=
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4=
Expand Down
53 changes: 37 additions & 16 deletions api/pkg/handlers/bulk_message_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
package handlers

import (
"crypto/rand"
"fmt"
"path/filepath"
"regexp"
"sync"
"sync/atomic"
"time"

"github.com/NdoleStudio/httpsms/pkg/requests"
"github.com/NdoleStudio/httpsms/pkg/services"
"github.com/NdoleStudio/httpsms/pkg/telemetry"
"github.com/NdoleStudio/httpsms/pkg/validators"
"github.com/davecgh/go-spew/spew"
"github.com/gofiber/fiber/v2"
gonanoid "github.com/matoous/go-nanoid/v2"
"github.com/palantir/stacktrace"
)

Expand Down Expand Up @@ -99,7 +100,7 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
return h.responseBadRequest(c, err)
}

messages, fileType, userLocation, validationErrors := h.validator.ValidateStore(ctx, h.userIDFomContext(c), file)
messages, userLocation, validationErrors := h.validator.ValidateStore(ctx, h.userIDFomContext(c), file)
if len(validationErrors) != 0 {
msg := fmt.Sprintf("validation errors [%s], while sending bulk sms from CSV file [%s] for [%s]", spew.Sdump(validationErrors), file.Filename, h.userIDFomContext(c))
ctxLogger.Warn(stacktrace.NewError(msg))
Expand All @@ -111,7 +112,7 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
return h.responsePaymentRequired(c, *msg)
}

requestID := h.generateRequestID(fileType, "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz")
requestID := h.generateRequestID(file.Filename)
wg := sync.WaitGroup{}
count := atomic.Int64{}

Expand Down Expand Up @@ -145,21 +146,41 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
return h.responseAccepted(c, fmt.Sprintf("Added %d out of %d messages to the queue", count.Load(), len(messages)))
}

func (h *BulkMessageHandler) generateRequestID(fileType string, alphabet string) string {
id, err := gonanoid.Generate(alphabet, 10)
if err != nil {
id = h.randomAlphaNum(10, alphabet)
func (h *BulkMessageHandler) generateRequestID(filename string) string {
return fmt.Sprintf("bulk-%s-%s", encodeBase62(time.Now().Unix()), truncateFilename(sanitizeFilename(filename), 32))
}

func sanitizeFilename(filename string) string {
return regexp.MustCompile(`[^a-zA-Z0-9.\-_: ]`).ReplaceAllString(filename, "")
}
Comment on lines +149 to +155
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Timestamp-only uniqueness — concurrent uploads collide

time.Now().Unix() has one-second granularity, so two uploads of the same filename that arrive within the same second produce an identical requestID. All messages from both batches would be tagged with the same ID and become indistinguishable in search and in the history table. The previous nanoid-based scheme avoided this by adding 10 random characters. Adding even 4 random alphanumeric characters after the timestamp (e.g. bulk-{ts}-{file}-{rand4}) would make collisions astronomically unlikely while keeping the human-readable filename benefit.

Copy link
Copy Markdown
Member Author

@AchoArnold AchoArnold May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Added a 4-character random base62 suffix after the timestamp, making the format \�ulk-{base62_ts}{rand4}-{filename}. This makes same-second collisions astronomically unlikely while keeping the human-readable filename.


func encodeBase62(n int64) string {
const charset = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
if n == 0 {
return "0"
}
result := make([]byte, 0, 8)
for n > 0 {
result = append(result, charset[n%62])
n /= 62
}
// reverse
for i, j := 0, len(result)-1; i < j; i, j = i+1, j-1 {
result[i], result[j] = result[j], result[i]
}
return fmt.Sprintf("bulk-%s-%s", fileType, id)
return string(result)
}

func (h *BulkMessageHandler) randomAlphaNum(length int, alphabet string) string {
b := make([]byte, length)
if _, err := rand.Read(b); err != nil {
return alphabet[:length]
func truncateFilename(filename string, maxLen int) string {
if len(filename) <= maxLen {
return filename
}
for i := range b {
b[i] = alphabet[int(b[i])%len(alphabet)]
ext := filepath.Ext(filename)
name := filename[:len(filename)-len(ext)]
available := maxLen - len(ext)
if available <= 0 {
return filename[:maxLen]
}
return string(b)
half := available / 2
return name[:half] + name[len(name)-(available-half):] + ext
}
Comment on lines +174 to 186
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unsanitized user-supplied filename embedded in requestID

truncateFilename preserves the raw filename characters (spaces, Unicode, dashes, dots, %, etc.) from the client's upload. The previous scheme used only a controlled string ("csv" or "xls"). If requestID is ever used in a log query, a URL parameter, or any downstream context that doesn't expect arbitrary text, the unsanitized portion could cause unexpected behavior. Consider stripping or replacing non-alphanumeric characters (excluding . and -) before embedding the filename in the ID.

Copy link
Copy Markdown
Member Author

@AchoArnold AchoArnold May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Added a sanitizeFilename function that strips all characters except alphanumeric, dots, and hyphens before embedding the filename in the request ID.

28 changes: 13 additions & 15 deletions api/pkg/validators/bulk_message_handler_validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewBulkMessageHandlerValidator(
}

// ValidateStore validates the requests.BillingUsageHistory request
func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID entities.UserID, header *multipart.FileHeader) ([]*requests.BulkMessage, string, *time.Location, url.Values) {
func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID entities.UserID, header *multipart.FileHeader) ([]*requests.BulkMessage, *time.Location, url.Values) {
ctx, span, ctxLogger := v.tracer.StartWithLogger(ctx, v.logger)
defer span.End()

Expand All @@ -61,22 +61,22 @@ func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID
result := url.Values{}
result.Add("document", "Cannot load your account. Please try again later or contact support.")
ctxLogger.Error(v.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot load user [%s]", userID))))
return nil, "", nil, result
return nil, nil, result
}

messages, fileType, result := v.parseFile(ctxLogger, user, header)
messages, result := v.parseFile(ctxLogger, user, header)
if len(result) != 0 {
return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

if len(messages) == 0 {
result.Add("document", "The uploaded file doesn't contain any valid records. Make sure you are using the official httpSMS template.")
return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

if len(messages) > 1000 {
result.Add("document", "The uploaded file must contain less than 1000 records.")
return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

for index, message := range messages {
Expand All @@ -85,32 +85,30 @@ func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID

result = v.validateMessages(ctx, messages, user.Location())
if len(result) != 0 {
return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

result = v.validateOwners(ctx, userID, messages)
if len(result) != 0 {
return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

return messages, fileType, user.Location(), result
return messages, user.Location(), result
}

func (v *BulkMessageHandlerValidator) parseFile(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, string, url.Values) {
func (v *BulkMessageHandlerValidator) parseFile(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, url.Values) {
if header.Header.Get("Content-Type") == "text/csv" || strings.HasSuffix(header.Filename, ".csv") {
messages, result := v.parseCSV(ctxLogger, user, header)
return messages, "csv", result
return v.parseCSV(ctxLogger, user, header)
}
if header.Header.Get("Content-Type") == "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" || strings.HasSuffix(header.Filename, ".xlsx") {
messages, result := v.parseXlsx(ctxLogger, user, header)
return messages, "xls", result
return v.parseXlsx(ctxLogger, user, header)
}

ctxLogger.Error(stacktrace.NewError(fmt.Sprintf("cannot parse file [%s] for user [%s] with content type [%s]", header.Filename, user.ID, header.Header.Get("Content-Type"))))

result := url.Values{}
result.Add("document", fmt.Sprintf("The file [%s] is not a valid CSV or Excel file.", header.Filename))
return nil, "", result
return nil, result
}

func (v *BulkMessageHandlerValidator) parseXlsx(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, url.Values) {
Expand Down
7 changes: 3 additions & 4 deletions web/pages/bulk-messages/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,9 @@ export default Vue.extend({
this.$store
.dispatch('sendBulkMessages', this.formFile)
.then(() => {
setTimeout(() => {
this.loading = false
this.$router.push({ name: 'threads' })
}, 2000)
this.loading = false
this.formFile = null
this.fetchBulkOrders()
})
.catch((error: AxiosError<ResponsesUnprocessableEntity>) => {
this.errorTitle = capitalize(
Expand Down
Loading