diff --git a/garbagecollect/collect_test.go b/garbagecollect/collect_test.go index 681b5764..a8605abc 100644 --- a/garbagecollect/collect_test.go +++ b/garbagecollect/collect_test.go @@ -39,13 +39,13 @@ func TestCollectExpiredFile(t *testing.T) { expireInFiveMins := mustParseExpirationTime("2025-01-01T00:05:00Z") dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("AAAAAAAAAAAA"), + ID: picoshare.MustCreateEntryID("AAAAAAAAAA"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("2024-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }) dataStore.InsertEntryDownload( - picoshare.EntryID("AAAAAAAAAAAA"), + picoshare.MustCreateEntryID("AAAAAAAAAA"), picoshare.DownloadRecord{ Time: mustParseTime("2023-06-01T12:00:00Z"), ClientIP: "192.168.1.1", @@ -53,28 +53,28 @@ func TestCollectExpiredFile(t *testing.T) { }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("BBBBBBBBBBBB"), + ID: picoshare.MustCreateEntryID("BBBBBBBBBB"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("3000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("CCCCCCCCCCCC"), + ID: picoshare.MustCreateEntryID("CCCCCCCCCC"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: picoshare.NeverExpire, Size: mustParseFileSize(len(d)), }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("DDDDDDDDDDDD"), + ID: picoshare.MustCreateEntryID("DDDDDDDDDD"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("2024-12-31T23:59:59Z"), Size: mustParseFileSize(len(d)), }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("EEEEEEEEEEEE"), + ID: picoshare.MustCreateEntryID("EEEEEEEEEE"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: expireInFiveMins, Size: mustParseFileSize(len(d)), @@ -93,19 +93,19 @@ func TestCollectExpiredFile(t *testing.T) { expected := []picoshare.UploadMetadata{ { - ID: picoshare.EntryID("BBBBBBBBBBBB"), + ID: picoshare.MustCreateEntryID("BBBBBBBBBB"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("3000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }, { - ID: picoshare.EntryID("CCCCCCCCCCCC"), + ID: picoshare.MustCreateEntryID("CCCCCCCCCC"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: picoshare.NeverExpire, Size: mustParseFileSize(len(d)), }, { - ID: picoshare.EntryID("EEEEEEEEEEEE"), + ID: picoshare.MustCreateEntryID("EEEEEEEEEE"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: expireInFiveMins, Size: mustParseFileSize(len(d)), @@ -125,21 +125,21 @@ func TestCollectDoesNothingWhenNoFilesAreExpired(t *testing.T) { d := "dummy data" dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("AAAAAAAAAAAA"), + ID: picoshare.MustCreateEntryID("AAAAAAAAAA"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("4000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("BBBBBBBBBBBB"), + ID: picoshare.MustCreateEntryID("BBBBBBBBBB"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("3000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }) dataStore.InsertEntry(strings.NewReader(d), picoshare.UploadMetadata{ - ID: picoshare.EntryID("CCCCCCCCCCCC"), + ID: picoshare.MustCreateEntryID("CCCCCCCCCC"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: picoshare.NeverExpire, Size: mustParseFileSize(len(d)), @@ -163,19 +163,19 @@ func TestCollectDoesNothingWhenNoFilesAreExpired(t *testing.T) { expected := []picoshare.UploadMetadata{ { - ID: picoshare.EntryID("AAAAAAAAAAAA"), + ID: picoshare.MustCreateEntryID("AAAAAAAAAA"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("4000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }, { - ID: picoshare.EntryID("BBBBBBBBBBBB"), + ID: picoshare.MustCreateEntryID("BBBBBBBBBB"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("3000-01-01T00:00:00Z"), Size: mustParseFileSize(len(d)), }, { - ID: picoshare.EntryID("CCCCCCCCCCCC"), + ID: picoshare.MustCreateEntryID("CCCCCCCCCC"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: picoshare.NeverExpire, Size: mustParseFileSize(len(d)), diff --git a/handlers/delete.go b/handlers/delete.go index 6b84a76f..f24e2650 100644 --- a/handlers/delete.go +++ b/handlers/delete.go @@ -6,11 +6,13 @@ import ( "net/http" "github.com/gorilla/mux" + + "github.com/mtlynch/picoshare/picoshare" ) func (s Server) entryDelete() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) diff --git a/handlers/delete_test.go b/handlers/delete_test.go index d0a88754..448aaca1 100644 --- a/handlers/delete_test.go +++ b/handlers/delete_test.go @@ -22,7 +22,7 @@ func TestDeleteExistingFile(t *testing.T) { fileContents := "dummy data" dataStore.InsertEntry(strings.NewReader(fileContents), picoshare.UploadMetadata{ - ID: picoshare.EntryID("hR87apiUCj"), + ID: picoshare.MustCreateEntryID("hR87apiUCj"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), Expires: mustParseExpirationTime("2024-01-01T00:00:00Z"), Size: mustParseFileSize(len(fileContents)), @@ -40,9 +40,9 @@ func TestDeleteExistingFile(t *testing.T) { status, http.StatusOK) } - _, err := dataStore.GetEntryMetadata(picoshare.EntryID("hR87apiUCj")) + _, err := dataStore.GetEntryMetadata(picoshare.MustCreateEntryID("hR87apiUCj")) if _, ok := err.(store.EntryNotFoundError); !ok { - t.Fatalf("expected entry %v to be deleted", picoshare.EntryID("hR87apiUCj")) + t.Fatalf("expected entry %v to be deleted", picoshare.MustCreateEntryID("hR87apiUCj")) } } diff --git a/handlers/download.go b/handlers/download.go index 7c39c1e8..88a75e72 100644 --- a/handlers/download.go +++ b/handlers/download.go @@ -28,7 +28,7 @@ type entryUnlockRequest struct { func (s Server) entryGet() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -60,7 +60,7 @@ func (s Server) entryGet() http.HandlerFunc { func (s Server) entryUnlockGet() http.HandlerFunc { t := parseTemplates("templates/pages/entry-unlock.html") return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -140,7 +140,7 @@ func (s Server) entryUnlockPost() http.HandlerFunc { } func parseEntryUnlockRequest(r *http.Request) (entryUnlockRequest, error) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { return entryUnlockRequest{}, err } diff --git a/handlers/download_test.go b/handlers/download_test.go index 560d277e..23f9dc68 100644 --- a/handlers/download_test.go +++ b/handlers/download_test.go @@ -14,7 +14,7 @@ import ( ) type mockEntry struct { - ID picoshare.EntryID + ID string Filename picoshare.Filename ContentType picoshare.ContentType } @@ -128,7 +128,7 @@ func TestEntryGet(t *testing.T) { data := "dummy data" entry := picoshare.UploadEntry{ UploadMetadata: picoshare.UploadMetadata{ - ID: mockEntry.ID, + ID: picoshare.MustCreateEntryID(mockEntry.ID), Filename: mockEntry.Filename, ContentType: mockEntry.ContentType, Uploaded: mustParseTime("2023-01-01T00:00:00Z"), @@ -176,7 +176,7 @@ func TestEntryGet(t *testing.T) { func TestProtectedEntryDownload(t *testing.T) { type fakeEntry struct { - ID picoshare.EntryID + ID string Contents string DownloadPassphrase picoshare.DownloadPassphrase } @@ -395,7 +395,7 @@ func TestProtectedEntryDownload(t *testing.T) { t.Run(tt.explanation, func(t *testing.T) { dataStore := test_sqlite.New(t) if err := dataStore.InsertEntry(strings.NewReader(tt.entryInStore.Contents), picoshare.UploadMetadata{ - ID: tt.entryInStore.ID, + ID: picoshare.MustCreateEntryID(tt.entryInStore.ID), Filename: "test.txt", ContentType: "text/plain", Uploaded: mustParseTime("2023-01-01T00:00:00Z"), @@ -448,7 +448,7 @@ func TestProtectedEntryDownloadRequiresPassphraseEveryDownload(t *testing.T) { dataStore := test_sqlite.New(t) data := "protected file contents" if err := dataStore.InsertEntry(strings.NewReader(data), picoshare.UploadMetadata{ - ID: "PPPPPPPPPP", + ID: picoshare.MustCreateEntryID("PPPPPPPPPP"), Filename: "protected.txt", ContentType: "text/plain", Uploaded: mustParseTime("2023-01-01T00:00:00Z"), diff --git a/handlers/guest_links_test.go b/handlers/guest_links_test.go index 7816c720..e34ac5fa 100644 --- a/handlers/guest_links_test.go +++ b/handlers/guest_links_test.go @@ -308,7 +308,7 @@ func TestDeleteExistingGuestLink(t *testing.T) { _, err := dataStore.GetGuestLink(picoshare.GuestLinkID("dummy-guest-link-id")) if _, ok := err.(store.GuestLinkNotFoundError); !ok { - t.Fatalf("expected entry %v to be deleted, got: %v", picoshare.EntryID("abcdefgh23456789"), err) + t.Fatalf("expected entry %v to be deleted, got: %v", "abcdefgh23456789", err) } } diff --git a/handlers/upload.go b/handlers/upload.go index cc27bfbb..9eed6f5e 100644 --- a/handlers/upload.go +++ b/handlers/upload.go @@ -12,15 +12,9 @@ import ( "github.com/mtlynch/picoshare/handlers/parse" "github.com/mtlynch/picoshare/picoshare" - "github.com/mtlynch/picoshare/random" "github.com/mtlynch/picoshare/store" ) -const EntryIDLength = 10 - -// Omit visually similar characters (I,l,1), (0,O) -var entryIDCharacters = []rune("abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNPQRSTUVWXYZ23456789") - type ( EntryPostResponse struct { ID string `json:"id"` @@ -69,7 +63,7 @@ func (s Server) entryPost() http.HandlerFunc { func (s Server) entryPut() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -208,35 +202,12 @@ func (s Server) entryMetadataFromRequest(r *http.Request) (picoshare.UploadMetad }, nil } -func generateEntryID() picoshare.EntryID { - return picoshare.EntryID(random.String(EntryIDLength, entryIDCharacters)) -} - -func parseEntryID(s string) (picoshare.EntryID, error) { - if len(s) != EntryIDLength { - return picoshare.EntryID(""), fmt.Errorf("entry ID (%v) has invalid length: got %d, want %d", s, len(s), EntryIDLength) - } - - // We could do this outside the function and store the result. - idCharsHash := map[rune]bool{} - for _, c := range entryIDCharacters { - idCharsHash[c] = true - } - - for _, c := range s { - if _, ok := idCharsHash[c]; !ok { - return picoshare.EntryID(""), fmt.Errorf("entry ID (%s) contains invalid character: %v", s, c) - } - } - return picoshare.EntryID(s), nil -} - func (s Server) insertFileFromRequest(r *http.Request, expiration picoshare.ExpirationTime, guestLinkID picoshare.GuestLinkID) (picoshare.EntryID, error) { // ParseMultipartForm can go above the limit we set, so set a conservative RAM // limit to avoid exhausting RAM on servers with limited resources. multipartMaxMemory := mibToBytes(1) if err := r.ParseMultipartForm(multipartMaxMemory); err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } defer func() { if err := r.MultipartForm.RemoveAll(); err != nil { @@ -246,45 +217,45 @@ func (s Server) insertFileFromRequest(r *http.Request, expiration picoshare.Expi reader, metadata, err := r.FormFile("file") if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } fileSize, err := picoshare.FileSizeFromInt64(metadata.Size) if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } filename, err := parse.Filename(metadata.Filename) if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } contentType, err := parseContentType(metadata.Header.Get("Content-Type")) if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } note, err := parse.FileNote(r.FormValue("note")) if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } if guestLinkID != "" && note.Value != nil { - return picoshare.EntryID(""), errors.New("guest uploads cannot have file notes") + return picoshare.EntryID{}, errors.New("guest uploads cannot have file notes") } downloadPassphrase := picoshare.DownloadPassphrase{} if rawDownloadPassphrase := r.FormValue("downloadPassphrase"); rawDownloadPassphrase != "" { if guestLinkID != "" { - return picoshare.EntryID(""), errors.New("guest uploads cannot have download passphrases") + return picoshare.EntryID{}, errors.New("guest uploads cannot have download passphrases") } downloadPassphrase, err = picoshare.NewDownloadPassphrase(rawDownloadPassphrase) if err != nil { - return picoshare.EntryID(""), err + return picoshare.EntryID{}, err } } - id := generateEntryID() + id := picoshare.NewEntryID() err = s.store.InsertEntry(reader, picoshare.UploadMetadata{ ID: id, @@ -301,7 +272,7 @@ func (s Server) insertFileFromRequest(r *http.Request, expiration picoshare.Expi }) if err != nil { log.Printf("failed to save entry: %v", err) - return picoshare.EntryID(""), dbError{err} + return picoshare.EntryID{}, dbError{err} } return id, nil diff --git a/handlers/upload_test.go b/handlers/upload_test.go index 499ad7c8..f3a6bd16 100644 --- a/handlers/upload_test.go +++ b/handlers/upload_test.go @@ -146,7 +146,7 @@ func TestEntryPost(t *testing.T) { t.Fatalf("response is not valid JSON: %v", body) } - entry, err := dataStore.GetEntryMetadata(picoshare.EntryID(response.ID)) + entry, err := dataStore.GetEntryMetadata(picoshare.MustCreateEntryID(response.ID)) if err != nil { t.Fatalf("failed to get expected entry %v from data store: %v", response.ID, err) } @@ -177,7 +177,7 @@ func TestEntryPost(t *testing.T) { func TestEntryPut(t *testing.T) { type fakeEntry struct { - ID picoshare.EntryID + ID string Filename picoshare.Filename Expires picoshare.ExpirationTime DownloadPassphrase string @@ -410,7 +410,7 @@ func TestEntryPut(t *testing.T) { dataStore := test_sqlite.New(t) originalData := "dummy original data" metadata := picoshare.UploadMetadata{ - ID: tt.entryInStore.ID, + ID: picoshare.MustCreateEntryID(tt.entryInStore.ID), Filename: tt.entryInStore.Filename, ContentType: picoshare.ContentType("audio/mpeg"), Uploaded: mustParseTime("2023-01-01T00:00:00Z"), @@ -439,7 +439,7 @@ func TestEntryPut(t *testing.T) { t.Fatalf("status=%d, want=%d", got, want) } - entry, err := dataStore.GetEntryMetadata(tt.entryInStore.ID) + entry, err := dataStore.GetEntryMetadata(picoshare.MustCreateEntryID(tt.entryInStore.ID)) if err != nil { t.Fatalf("failed to get expected entry %v from data store: %v", tt.entryInStore.ID, err) } @@ -596,7 +596,7 @@ func TestGuestUpload(t *testing.T) { entriesInStore: []picoshare.UploadEntry{ { UploadMetadata: picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-entry1"), + ID: picoshare.MustCreateEntryID("AAAAAAAAAA"), Uploaded: mustParseTime("2024-02-01T00:00:00Z"), GuestLink: picoshare.GuestLink{ ID: picoshare.GuestLinkID("abcdefgh23456789"), @@ -606,7 +606,7 @@ func TestGuestUpload(t *testing.T) { }, { UploadMetadata: picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-entry2"), + ID: picoshare.MustCreateEntryID("BBBBBBBBBB"), Uploaded: mustParseTime("2024-02-02T00:00:00Z"), GuestLink: picoshare.GuestLink{ ID: picoshare.GuestLinkID("abcdefgh23456789"), @@ -817,7 +817,7 @@ func TestGuestUpload(t *testing.T) { t.Fatalf("response is not valid JSON: %v", body) } - entry, err := dataStore.GetEntryMetadata(picoshare.EntryID(response.ID)) + entry, err := dataStore.GetEntryMetadata(picoshare.MustCreateEntryID(response.ID)) if err != nil { t.Fatalf("failed to get expected entry %v from data store: %v", response.ID, err) } diff --git a/handlers/views.go b/handlers/views.go index 7c91fa06..3a02fa68 100644 --- a/handlers/views.go +++ b/handlers/views.go @@ -215,7 +215,7 @@ func (s Server) fileEditGet() http.HandlerFunc { "templates/pages/file-edit.html") return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -268,7 +268,7 @@ func (s Server) fileInfoGet() http.HandlerFunc { "templates/pages/file-info.html") return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -316,7 +316,7 @@ func (s Server) fileDownloadsGet() http.HandlerFunc { t := parseTemplatesWithFuncs(fns, "templates/pages/file-downloads.html") return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) @@ -394,7 +394,7 @@ func (s Server) fileConfirmDeleteGet() http.HandlerFunc { t := parseTemplates("templates/pages/file-delete.html") return func(w http.ResponseWriter, r *http.Request) { - id, err := parseEntryID(mux.Vars(r)["id"]) + id, err := picoshare.EntryIDFromString(mux.Vars(r)["id"]) if err != nil { log.Printf("error parsing ID: %v", err) http.Error(w, fmt.Sprintf("bad entry ID: %v", err), http.StatusBadRequest) diff --git a/picoshare/entry_id_test.go b/picoshare/entry_id_test.go new file mode 100644 index 00000000..2ef37bc8 --- /dev/null +++ b/picoshare/entry_id_test.go @@ -0,0 +1,73 @@ +package picoshare_test + +import ( + "strings" + "testing" + + "github.com/mtlynch/picoshare/picoshare" +) + +func TestEntryIDFromString(t *testing.T) { + for _, tt := range []struct { + explanation string + input string + isValidExpected bool + }{ + { + explanation: "ten allowed characters are valid", + input: strings.Repeat("a", picoshare.EntryIDLength), + isValidExpected: true, + }, + { + explanation: "an ID shorter than ten characters is invalid", + input: strings.Repeat("a", picoshare.EntryIDLength-1), + isValidExpected: false, + }, + { + explanation: "an ID longer than ten characters is invalid", + input: strings.Repeat("a", picoshare.EntryIDLength+1), + isValidExpected: false, + }, + { + explanation: "a visually ambiguous uppercase I is invalid", + input: strings.Repeat("a", picoshare.EntryIDLength-1) + "I", + isValidExpected: false, + }, + { + explanation: "a visually ambiguous lowercase l is invalid", + input: strings.Repeat("a", picoshare.EntryIDLength-1) + "l", + isValidExpected: false, + }, + { + explanation: "a Unicode character is invalid", + input: strings.Repeat("a", picoshare.EntryIDLength-1) + "é", + isValidExpected: false, + }, + } { + t.Run(tt.explanation, func(t *testing.T) { + id, err := picoshare.EntryIDFromString(tt.input) + isValid := err == nil + if got, want := isValid, tt.isValidExpected; got != want { + t.Fatalf("EntryIDFromString validity=%t, want=%t", got, want) + } + if !isValid { + return + } + if got, want := id.String(), tt.input; got != want { + t.Errorf("ID=%q, want=%q", got, want) + } + }) + } +} + +func TestNewEntryID(t *testing.T) { + id := picoshare.NewEntryID() + if got, want := len(id.String()), picoshare.EntryIDLength; got != want { + t.Errorf("length=%d, want=%d", got, want) + } + + _, err := picoshare.EntryIDFromString(id.String()) + if err != nil { + t.Errorf("failed to parse generated entry ID: %v", err) + } +} diff --git a/picoshare/picoshare.go b/picoshare/picoshare.go index b7493503..3e9b25b2 100644 --- a/picoshare/picoshare.go +++ b/picoshare/picoshare.go @@ -1,12 +1,14 @@ package picoshare import ( + "fmt" "io" "time" + + "github.com/mtlynch/picoshare/random" ) type ( - EntryID string Filename string ContentType string ExpirationTime time.Time @@ -46,11 +48,60 @@ type ( } ) +// EntryIDLength is the number of characters in an entry ID. +const EntryIDLength = 10 + +// entryIDCharacters omits visually similar characters (I, l, 1), (0, O). +const entryIDCharacters = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNPQRSTUVWXYZ23456789" + +var entryIDCharacterSet = func() map[rune]struct{} { + characters := make(map[rune]struct{}, len(entryIDCharacters)) + for _, character := range entryIDCharacters { + characters[character] = struct{}{} + } + return characters +}() + +// EntryID identifies an uploaded entry. +type EntryID struct { + value string +} + // Treat a distant expiration time as sort of a sentinel value signifying a "never expire" option. var NeverExpire = ExpirationTime(time.Date(2999, time.December, 31, 0, 0, 0, 0, time.UTC)) func (id EntryID) String() string { - return string(id) + return id.value +} + +// EntryIDFromString constructs an entry ID from user-provided text. +func EntryIDFromString(raw string) (EntryID, error) { + if len(raw) != EntryIDLength { + return EntryID{}, fmt.Errorf( + "entry ID has invalid length: got %d, want %d", len(raw), EntryIDLength) + } + + for _, character := range raw { + if _, ok := entryIDCharacterSet[character]; !ok { + return EntryID{}, fmt.Errorf("entry ID contains invalid character: %q", character) + } + } + + return EntryID{value: raw}, nil +} + +// NewEntryID generates an entry ID. +func NewEntryID() EntryID { + return EntryID{value: random.String(EntryIDLength, []rune(entryIDCharacters))} +} + +// MustCreateEntryID constructs an entry ID or panics when raw is invalid. +func MustCreateEntryID(raw string) EntryID { + id, err := EntryIDFromString(raw) + if err != nil { + panic(fmt.Sprintf("failed to create entry ID: %v", err)) + } + return id } func (f Filename) String() string { diff --git a/store/sqlite/downloads.go b/store/sqlite/downloads.go index 6dc49caa..0c51afa1 100644 --- a/store/sqlite/downloads.go +++ b/store/sqlite/downloads.go @@ -41,7 +41,7 @@ func (s Store) GetEntryDownloads(id picoshare.EntryID) ([]picoshare.DownloadReco WHERE entry_id=:entry_id ORDER BY - download_timestamp DESC`, sql.Named("entry_id", id)) + download_timestamp DESC`, sql.Named("entry_id", id.String())) if err == sql.ErrNoRows { return []picoshare.DownloadRecord{}, nil } else if err != nil { diff --git a/store/sqlite/entries.go b/store/sqlite/entries.go index 60fc2783..013f9ee2 100644 --- a/store/sqlite/entries.go +++ b/store/sqlite/entries.go @@ -3,6 +3,7 @@ package sqlite import ( "context" "database/sql" + "fmt" "io" "log" @@ -49,6 +50,10 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) { if err = rows.Scan(&id, &filename, ¬e, &contentType, &uploadTimeRaw, &expirationTimeRaw, &fileSizeRaw); err != nil { return []picoshare.UploadMetadata{}, err } + entryID, err := picoshare.EntryIDFromString(id) + if err != nil { + return []picoshare.UploadMetadata{}, fmt.Errorf("failed to parse entry ID from database: %w", err) + } ut, err := parseDatetime(uploadTimeRaw) if err != nil { @@ -66,7 +71,7 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) { } ee = append(ee, picoshare.UploadMetadata{ - ID: picoshare.EntryID(id), + ID: entryID, Filename: picoshare.Filename(filename), Note: picoshare.FileNote{Value: note}, ContentType: picoshare.ContentType(contentType), @@ -120,7 +125,7 @@ func (s Store) GetEntryMetadata(id picoshare.EntryID) (picoshare.UploadMetadata, id ) sizes ON entries.id = sizes.id WHERE - entries.id = :entry_id`, sql.Named("entry_id", id)).Scan(&filename, ¬e, &contentType, &uploadTimeRaw, &expirationTimeRaw, &downloadPassphraseRaw, &fileSizeRaw, &guestLinkID) + entries.id = :entry_id`, sql.Named("entry_id", id.String())).Scan(&filename, ¬e, &contentType, &uploadTimeRaw, &expirationTimeRaw, &downloadPassphraseRaw, &fileSizeRaw, &guestLinkID) if err == sql.ErrNoRows { return picoshare.UploadMetadata{}, store.EntryNotFoundError{ID: id} } else if err != nil { @@ -198,7 +203,7 @@ func (s Store) InsertEntry(reader io.Reader, metadata picoshare.UploadMetadata) download_passphrase ) VALUES(:entry_id, NULLIF(:guest_link_id, ''), :filename, :note, :content_type, :upload_time, :expiration_time, :download_passphrase)`, - sql.Named("entry_id", metadata.ID), + sql.Named("entry_id", metadata.ID.String()), sql.Named("guest_link_id", metadata.GuestLink.ID), sql.Named("filename", metadata.Filename), sql.Named("note", metadata.Note.Value), @@ -246,7 +251,7 @@ func (s Store) UpdateEntryMetadata(id picoshare.EntryID, metadata picoshare.Uplo sql.Named("expiration_time", formatExpirationTime(metadata.Expires)), sql.Named("note", metadata.Note.Value), sql.Named("download_passphrase", downloadPassphraseString(metadata.DownloadPassphrase)), - sql.Named("entry_id", id)) + sql.Named("entry_id", id.String())) if err != nil { return err } @@ -280,7 +285,7 @@ func (s Store) DeleteEntry(id picoshare.EntryID) error { DELETE FROM downloads WHERE - entry_id = :entry_id`, sql.Named("entry_id", id)); err != nil { + entry_id = :entry_id`, sql.Named("entry_id", id.String())); err != nil { log.Printf("delete from downloads table failed, aborting transaction: %v", err) return err } @@ -289,7 +294,7 @@ func (s Store) DeleteEntry(id picoshare.EntryID) error { DELETE FROM entries_data WHERE - id = :entry_id`, sql.Named("entry_id", id)); err != nil { + id = :entry_id`, sql.Named("entry_id", id.String())); err != nil { log.Printf("delete from entries_data table failed, aborting transaction: %v", err) return err } @@ -298,7 +303,7 @@ func (s Store) DeleteEntry(id picoshare.EntryID) error { DELETE FROM entries WHERE - id = :entry_id`, sql.Named("entry_id", id)); err != nil { + id = :entry_id`, sql.Named("entry_id", id.String())); err != nil { log.Printf("delete from entries table failed, aborting transaction: %v", err) return err } diff --git a/store/sqlite/entries_test.go b/store/sqlite/entries_test.go index e87da9cd..e20ceaf9 100644 --- a/store/sqlite/entries_test.go +++ b/store/sqlite/entries_test.go @@ -18,7 +18,7 @@ func TestInsertDeleteSingleEntry(t *testing.T) { input := "hello, world!" if err := dataStore.InsertEntry(bytes.NewBufferString(input), picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-id"), + ID: picoshare.MustCreateEntryID("abcdefghij"), Filename: "dummy-file.txt", Uploaded: mustParseTime("2025-05-25T00:00:00Z"), Expires: mustParseExpirationTime("2040-01-01T00:00:00Z"), @@ -27,7 +27,7 @@ func TestInsertDeleteSingleEntry(t *testing.T) { t.Fatalf("failed to insert file into sqlite: %v", err) } - entryFile, err := dataStore.ReadEntryFile("dummy-id") + entryFile, err := dataStore.ReadEntryFile(picoshare.MustCreateEntryID("abcdefghij")) if err != nil { t.Fatalf("failed to get entry from DB: %v", err) } @@ -62,7 +62,7 @@ func TestInsertDeleteSingleEntry(t *testing.T) { t.Fatalf("filename=%s, want=%s", got, want) } - err = dataStore.DeleteEntry(picoshare.EntryID("dummy-id")) + err = dataStore.DeleteEntry(picoshare.MustCreateEntryID("abcdefghij")) if err != nil { t.Fatalf("failed to delete entry: %v", err) } @@ -83,7 +83,7 @@ func TestReadLastByteOfEntry(t *testing.T) { input := "hello, world!" if err := db.InsertEntry(bytes.NewBufferString(input), picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-id"), + ID: picoshare.MustCreateEntryID("abcdefghij"), Filename: "dummy-file.txt", Uploaded: mustParseTime("2025-05-25T00:00:00Z"), Expires: mustParseExpirationTime("2040-01-01T00:00:00Z"), @@ -92,7 +92,7 @@ func TestReadLastByteOfEntry(t *testing.T) { t.Fatalf("failed to insert file into sqlite: %v", err) } - entryFile, err := db.ReadEntryFile(picoshare.EntryID("dummy-id")) + entryFile, err := db.ReadEntryFile(picoshare.MustCreateEntryID("abcdefghij")) if err != nil { t.Fatalf("failed to read entry: %v", err) } @@ -126,7 +126,7 @@ func TestUpdateEntryMetadata(t *testing.T) { data := "dummy data" if err := dataStore.InsertEntry(strings.NewReader(data), picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-id"), + ID: picoshare.MustCreateEntryID("abcdefghij"), Filename: "dummy-file.txt", Uploaded: mustParseTime("2025-05-25T00:00:00Z"), Expires: mustParseExpirationTime("2040-01-01T00:00:00Z"), @@ -136,7 +136,7 @@ func TestUpdateEntryMetadata(t *testing.T) { t.Fatalf("failed to insert file into sqlite: %v", err) } - metadata, err := dataStore.GetEntryMetadata("dummy-id") + metadata, err := dataStore.GetEntryMetadata(picoshare.MustCreateEntryID("abcdefghij")) if err != nil { t.Fatalf("failed to retrieve entry metadata: %v", err) } @@ -144,7 +144,7 @@ func TestUpdateEntryMetadata(t *testing.T) { t.Errorf("download passphrase=%q, want=%q", got, want) } - if err := dataStore.UpdateEntryMetadata("dummy-id", picoshare.UploadMetadata{ + if err := dataStore.UpdateEntryMetadata(picoshare.MustCreateEntryID("abcdefghij"), picoshare.UploadMetadata{ Filename: "renamed-file.txt", Expires: mustParseExpirationTime("2041-01-01T00:00:00Z"), Note: picoshare.FileNote{Value: new("updated note")}, @@ -153,7 +153,7 @@ func TestUpdateEntryMetadata(t *testing.T) { t.Fatalf("failed to update entry metadata: %v", err) } - metadata, err = dataStore.GetEntryMetadata("dummy-id") + metadata, err = dataStore.GetEntryMetadata(picoshare.MustCreateEntryID("abcdefghij")) if err != nil { t.Fatalf("failed to retrieve entry metadata: %v", err) } @@ -183,7 +183,7 @@ func TestGetEntriesMetadataOmitsDownloadPassphrase(t *testing.T) { data := "dummy data" if err := dataStore.InsertEntry(strings.NewReader(data), picoshare.UploadMetadata{ - ID: picoshare.EntryID("dummy-id"), + ID: picoshare.MustCreateEntryID("abcdefghij"), Filename: "dummy-file.txt", Uploaded: mustParseTime("2025-05-25T00:00:00Z"), Expires: mustParseExpirationTime("2040-01-01T00:00:00Z"), diff --git a/store/sqlite/file/reader.go b/store/sqlite/file/reader.go index c3b6581c..c4cbcbaf 100644 --- a/store/sqlite/file/reader.go +++ b/store/sqlite/file/reader.go @@ -100,7 +100,7 @@ func (fr *fileReader) populateBuffer() error { ORDER BY chunk_index ASC `, - sql.Named("id", fr.entryID), + sql.Named("id", fr.entryID.String()), sql.Named("chunk_index", chunkIndex), ).Scan(&chunk); err != nil { log.Printf("reading chunk failed: %v", err) @@ -131,7 +131,7 @@ func getFileLength(db *sql.DB, id picoshare.EntryID, chunkSize int64) (int64, er chunk_index DESC LIMIT 1 `, - sql.Named("id", id), + sql.Named("id", id.String()), ).Scan(&chunkIndex, &chunkLen); err != nil { return 0, err } @@ -155,7 +155,7 @@ func getChunkSize(db *sql.DB, id picoshare.EntryID) (int64, error) { chunk_index ASC LIMIT 1 `, - sql.Named("id", id), + sql.Named("id", id.String()), ).Scan(&chunkSize); err != nil { return 0, err } diff --git a/store/sqlite/file/writer.go b/store/sqlite/file/writer.go index 7412aaae..0414bb12 100644 --- a/store/sqlite/file/writer.go +++ b/store/sqlite/file/writer.go @@ -75,7 +75,7 @@ func (w *writer) flush(n int) error { chunk ) VALUES(:id,:chunk_index,:chunk)`, - sql.Named("id", w.entryID), + sql.Named("id", w.entryID.String()), sql.Named("chunk_index", idx), sql.Named("chunk", w.buf[0:n]), ) diff --git a/store/sqlite/file/writer_test.go b/store/sqlite/file/writer_test.go index f2f6e148..875c64f8 100644 --- a/store/sqlite/file/writer_test.go +++ b/store/sqlite/file/writer_test.go @@ -26,7 +26,10 @@ type ( var errMockSqlFailure = errors.New("fake SQL error") func (db *mockSqlDB) Exec(query string, args ...any) (sql.Result, error) { - id := args[0].(sql.NamedArg).Value.(picoshare.EntryID) + id, err := picoshare.EntryIDFromString(args[0].(sql.NamedArg).Value.(string)) + if err != nil { + return nil, err + } chunkIndex := args[1].(sql.NamedArg).Value.(int) chunk := args[2].(sql.NamedArg).Value.([]byte) chunkCopy := make([]byte, len(chunk)) @@ -51,12 +54,12 @@ func TestWriteFile(t *testing.T) { }{ { explanation: "data is smaller than chunk size", - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), data: []byte("hello, world!"), chunkSize: 25, rowsExpected: []mockChunkRow{ { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 0, chunk: []byte("hello, world!"), }, @@ -64,12 +67,12 @@ func TestWriteFile(t *testing.T) { }, { explanation: "data fits exactly in single chunk", - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), data: []byte("01234"), chunkSize: 5, rowsExpected: []mockChunkRow{ { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 0, chunk: []byte("01234"), }, @@ -77,17 +80,17 @@ func TestWriteFile(t *testing.T) { }, { explanation: "data occupies a partial chunk after the first", - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), data: []byte("0123456"), chunkSize: 5, rowsExpected: []mockChunkRow{ { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 0, chunk: []byte("01234"), }, { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 1, chunk: []byte("56"), }, @@ -95,17 +98,17 @@ func TestWriteFile(t *testing.T) { }, { explanation: "data spans exactly two chunks", - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), data: []byte("0123456789"), chunkSize: 5, rowsExpected: []mockChunkRow{ { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 0, chunk: []byte("01234"), }, { - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), chunkIndex: 1, chunk: []byte("56789"), }, @@ -113,7 +116,7 @@ func TestWriteFile(t *testing.T) { }, { explanation: "write fails when SQL transaction returns error", - id: picoshare.EntryID("dummy-id"), + id: picoshare.MustCreateEntryID("abcdefghij"), data: []byte("0123456789"), chunkSize: 5, sqlExecErr: errMockSqlFailure,