Skip to content

Commit

Permalink
fix(expiration): show user errors under file upload form instead of r…
Browse files Browse the repository at this point in the history
…edirecting
  • Loading branch information
benjohns1 committed Feb 12, 2024
1 parent 27cb93c commit 8d7a5f7
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 21 deletions.
3 changes: 3 additions & 0 deletions app/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ func (a *App) UploadFile(ctx context.Context, args UploadFileArgs) error {
DownloadLimit: args.DownloadLimit,
})
if err != nil {
if errors.Is(err, blinkfile.ErrExpirationInPast) {
return ErrUser("Error uploading file", "Cannot upload a file that expires in the past.", err)
}
return Err(ErrBadRequest, err)
}
err = a.cfg.FileRepo.Save(ctx, file)
Expand Down
41 changes: 29 additions & 12 deletions app/web/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,33 @@ func formatFileSize(size int64) string {
}

func uploadFile(ctx iris.Context, a App) error {
filename, err := doFileUpload(ctx, a)
if err != nil {
setFlashErr(ctx, a, err)
} else {
setFlashSuccess(ctx, fmt.Sprintf("Successfully uploaded %s", filename))
}
ctx.Redirect("/")
return nil
}

func doFileUpload(ctx iris.Context, a App) (string, error) {
args, err := parseFileUploadArgs(ctx)
if err != nil {
return "", err
}
err = a.UploadFile(ctx, args)
if err != nil {
return "", err
}
return args.Filename, nil
}

func parseFileUploadArgs(ctx iris.Context) (app.UploadFileArgs, error) {
var empty app.UploadFileArgs
file, header, err := ctx.FormFile("file")
if err != nil {
return app.ErrUser("Invalid file.", "We couldn't retrieve the uploaded file, please try again.", err)
return empty, app.ErrUser("Invalid file.", "We couldn't retrieve the uploaded file, please try again.", err)
}

var expiresIn longduration.LongDuration
Expand All @@ -108,18 +132,18 @@ func uploadFile(ctx iris.Context, a App) error {
expires, err = time.Parse(time.RFC3339, expirationTime)
if err != nil {
err = fmt.Errorf("parsing expiration time %q: %w", expirationTime, err)
return app.ErrUser("Invalid expiration time.", fmt.Sprintf("We couldn't understand the file expiration time %q, please make sure the date format is correct.", expirationTime), err)
return empty, app.ErrUser("Invalid expiration time.", fmt.Sprintf("We couldn't understand the file expiration time %q, please make sure the date format is correct.", expirationTime), err)
}
}
var downloadLimit int64
downloadLimitStr := ctx.FormValue("download_limit")
if downloadLimitStr != "" {
_, err = fmt.Sscan(downloadLimitStr, &downloadLimit)
if err != nil {
return app.ErrUser("Invalid download limit.", "Invalid file download limit, please make sure it's a valid number.", err)
return empty, app.ErrUser("Invalid download limit.", "Invalid file download limit, please make sure it's a valid number.", err)
}
}
args := app.UploadFileArgs{
return app.UploadFileArgs{
Filename: header.Filename,
Owner: loggedInUser(ctx),
Reader: file,
Expand All @@ -128,14 +152,7 @@ func uploadFile(ctx iris.Context, a App) error {
ExpiresIn: expiresIn,
Expires: expires,
DownloadLimit: downloadLimit,
}
err = a.UploadFile(ctx, args)
if err != nil {
return err
}
setFlashSuccess(ctx, fmt.Sprintf("Successfully uploaded %s", header.Filename))
ctx.Redirect("/")
return nil
}, nil
}

func sanitizeFilename(in string) string {
Expand Down
10 changes: 10 additions & 0 deletions app/web/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ func setFlashSuccess(ctx iris.Context, msg string) {
sess.SetFlash(flashMessageKey, view)
}

func setFlashErr(ctx iris.Context, a App, err error) {
sess := sessions.Get(ctx)
if sess == nil {
return
}
view := MessageView{ErrorView: ParseAppErr(ctx, a, err)}

sess.SetFlash(flashMessageKey, view)
}

func flashMessageView(ctx iris.Context) MessageView {
sess := sessions.Get(ctx)
if sess == nil {
Expand Down
3 changes: 2 additions & 1 deletion file.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func UploadFile(args UploadFileArgs) (file File, err error) {
expires = args.Expires
}
if !expires.IsZero() && !expires.After(now) {
return File{}, fmt.Errorf("expiration cannot be set in the past")
return File{}, ErrExpirationInPast
}
if args.DownloadLimit < 0 {
return File{}, fmt.Errorf("download limit cannot be negative")
Expand All @@ -101,6 +101,7 @@ var (
ErrFilePasswordInvalid = fmt.Errorf("invalid file password")
ErrFileExpired = fmt.Errorf("file has expired")
ErrDownloadLimitReached = fmt.Errorf("file download limit reached")
ErrExpirationInPast = fmt.Errorf("expiration cannot be set in the past")
)

func (f *FileHeader) Download(user UserID, password string, matchFunc PasswordMatchFunc, nowFunc NowFunc) (err error) {
Expand Down
8 changes: 7 additions & 1 deletion test/cypress/features/expiration.feature
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ Scenario: A file is removed after it expires
And I can successfully download the file
When "3 days" has passed
Then I can no longer download the file
And it no longer shows up in the file list
And it no longer shows up in the file list

Scenario: Cannot upload a file that expires in the past
Given I have selected the file "files/expiration.txt" to upload
When I set it to expire in "-1 minutes"
And I try to upload the file
Then I should see an error message that contains "Cannot upload a file that expires in the past"
31 changes: 24 additions & 7 deletions test/cypress/steps/expiration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import {
getFileExpirations,
getExpiresInField,
getExpiresInUnitField,
verifyFileResponse, visitFileListPage, fileRowsSelector, cannotDownloadFileNoPassword, fileNotInList
verifyFileResponse,
visitFileListPage,
fileRowsSelector,
cannotDownloadFileNoPassword,
fileNotInList,
shouldSeeUploadFailureMessage
} from "./shared/files";
import {login, logout} from "./shared/login";
import dayjs from "dayjs";
Expand Down Expand Up @@ -65,11 +70,17 @@ When("I enter {string} for the expiration date", (date: string) => {
const expireIn = (expiresIn: string) => {
let expiresInAmount: string;
let expiresInUnit: string;
if (expiresIn === "3 days") {
expiresInAmount = "3";
expiresInUnit = "d";
} else {
throw `expiresIn string ${expiresIn} not implemented`;
switch (expiresIn) {
case "3 days":
expiresInAmount = "3";
expiresInUnit = "d";
break;
case "-1 minutes":
expiresInAmount = "-1";
expiresInUnit = "m";
break;
default:
throw `expiresIn string ${expiresIn} not implemented`;
}
getExpiresInField().type(expiresInAmount);
getExpiresInUnitField().select(expiresInUnit);
Expand All @@ -93,6 +104,10 @@ When("I upload the file", () => {
uploadSelectedFile();
});

When("I try to upload the file", () => {
getUploadButton().click();
});

Then("it should upload successfully", () => {
shouldSeeUploadSuccessMessage(state.fileToUpload);
const filename = filepathBase(state.fileToUpload);
Expand Down Expand Up @@ -164,4 +179,6 @@ Then("I can no longer download the file", () => {
Then("it no longer shows up in the file list", () => {
visitFileListPage();
fileNotInList(state.fileLink);
});
});

Then("I should see an error message that contains {string}", shouldSeeUploadFailureMessage);
4 changes: 4 additions & 0 deletions test/cypress/steps/shared/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ export const shouldSeeUploadSuccessMessage = (filepath: string) => {
getMessage().should("contain", `Successfully uploaded ${filename}`);
};

export const shouldSeeUploadFailureMessage = (contains: string) => {
getMessage().should("contain", contains);
};

export const cannotDownloadFileNoPassword = (link: string) => {
cy.request(link).then(response => {
expect(response.headers["content-type"]).to.contain("text/html");
Expand Down

0 comments on commit 8d7a5f7

Please sign in to comment.