diff --git a/assets/js/uploader-submitter.mjs b/assets/js/uploader-submitter.mjs index b61289de..375e6e57 100644 --- a/assets/js/uploader-submitter.mjs +++ b/assets/js/uploader-submitter.mjs @@ -5,6 +5,13 @@ import { getElmById, validId } from "./helper.mjs"; export default submitter; +/** + * To test some error handling, use the following IDs: + * + * Pouët ID: 16, deleted from the database + * Pouët ID: 15, not suitable for Defacto2 + */ + const invalid = "is-invalid", none = "d-none"; @@ -44,19 +51,31 @@ export function submitter(elementId, api) { break; } + // The htmx:beforeRequest event is triggered before the request is made. document.body.addEventListener("htmx:beforeRequest", function () { beforeReset(alert, results); }); - document.body.addEventListener("htmx:afterRequest", function (event) { - if (event.detail.elt === null || event.detail.elt.id !== `${elementId}`) { + // The htmx:beforeSwap event is triggered before the content is swapped. + // This is the best place to check the status of the request and display an error message. + document.body.addEventListener("htmx:beforeSwap", function (evt) { + const badRequest = 400; + if (evt.detail.xhr.status >= badRequest) { + alert.classList.remove(none); + } + }); + + // The htmx:afterRequest event is triggered after the request is completed. + // Multiple requests can be made, so we need to check if the request is the one we are interested in. + document.body.addEventListener("htmx:afterRequest", function (evt) { + if (evt.detail.elt === null || evt.detail.elt.id !== `${elementId}`) { return; } - if (event.detail.successful) { - return successful(alert); + if (evt.detail.successful) { + return successful(input); } - const xhr = event.detail.xhr; - if (event.detail.failed && xhr) { + const xhr = evt.detail.xhr; + if (evt.detail.failed && xhr) { if (xhr.status === 404) { return error404(alert, results, api); } @@ -82,7 +101,9 @@ function beforeReset(alert, results) { alert.classList.add(none); } -function successful() {} +function successful(input) { + input.focus(); +} function error404(alert, results, api) { results.innerText = `Production not found on ${api}.`; diff --git a/docs/todo.md b/docs/todo.md index dd4c1e01..42afd425 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -3,8 +3,6 @@ ### Stuff to do * Repack zips that contain programs with bad filenames, for example: http://localhost:1323/f/ab252e4 -* Fix (or remove) Pouet submission, it is not at all working. -* Review all comments for functions etc. * _Replacement image_ *ALWAYS* prompts for a confirmation. This should only occur if there is an existing image. * _Thumbnail assets_ links need better separation. * `
` diff --git a/handler/app/remote/remote.go b/handler/app/remote/remote.go index 832527d0..9cbbe074 100644 --- a/handler/app/remote/remote.go +++ b/handler/app/remote/remote.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/Defacto2/archive" "github.com/Defacto2/helper" @@ -70,7 +71,7 @@ func (got *DemozooLink) Download(c echo.Context, db *sql.DB, downloadDir string) if link.URL == "" { continue } - df, err := helper.GetFile(link.URL) + df, err := GetFile(link.URL, 0) if tryNextLink := err != nil || df.Path == ""; tryNextLink { continue } @@ -173,7 +174,7 @@ func (got DemozooLink) Update(c echo.Context, db *sql.DB) error { if err = tx.Commit(); err != nil { return fmt.Errorf("demozoolink update commit %w: %s", err, uid) } - return c.HTML(http.StatusOK, `New artifact update, okay
`) + return c.HTML(http.StatusOK, `Successful Demozoo update
`) } func (got DemozooLink) updates(f *models.File) { //nolint:cyclop @@ -271,7 +272,7 @@ func (got *PouetLink) Download(c echo.Context, db *sql.DB, downloadDir string) e if downloadURL == "" { return nil } - df, err := helper.GetFile(downloadURL) + df, err := GetFile(downloadURL, 10*time.Second) if err != nil { return fmt.Errorf("could not get file, %s: %w", downloadURL, err) } @@ -361,7 +362,7 @@ func (got PouetLink) Update(c echo.Context, db *sql.DB) error { if err = tx.Commit(); err != nil { return fmt.Errorf("demozoolink update commit %w: %s", err, uid) } - return c.HTML(http.StatusOK, `New artifact update, okay
`) + return c.HTML(http.StatusOK, `Successful Pouet update
`) } func (got PouetLink) updates(f *models.File) { diff --git a/handler/app/remote/remote_test.go b/handler/app/remote/remote_test.go index b1d1ad38..c1536cd5 100644 --- a/handler/app/remote/remote_test.go +++ b/handler/app/remote/remote_test.go @@ -4,8 +4,10 @@ package remote_test import ( "testing" + "time" "github.com/Defacto2/server/handler/app/remote" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -32,3 +34,22 @@ func TestUpdate(t *testing.T) { err := dl.Update(nil, nil) require.Error(t, err) } + +func TestFixSceneOrg(t *testing.T) { + s := "http://files.scene.org/view/demos/groups/trsi/ms-dos/trsiscxt.zip" + w := remote.FixSceneOrg(s) + assert.Equal(t, "https://files.scene.org/get/demos/groups/trsi/ms-dos/trsiscxt.zip", w) +} + +func TestGetExampleCom(t *testing.T) { + t.Parallel() + const testDefaultTimeout = 0 + r, err := remote.GetFile("http://example.com", testDefaultTimeout) + assert.NotEqual(t, "", r.Path) + assert.Equal(t, "text/html; charset=UTF-8", r.ContentType) + require.NoError(t, err) + + const invalidTimeout = 1 * time.Microsecond + _, err = remote.GetFile("http://example.com", invalidTimeout) + require.Error(t, err) +} diff --git a/handler/app/remote/request.go b/handler/app/remote/request.go new file mode 100644 index 00000000..42d4f684 --- /dev/null +++ b/handler/app/remote/request.go @@ -0,0 +1,108 @@ +package remote + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "os" + "strings" + "time" + + "github.com/Defacto2/helper" +) + +const ( + // Timeout is the HTTP client default timeout. + Timeout = 5 * time.Second + // User-Agent to send with the HTTP request. + UserAgent = "Defacto2 Uploader form submission, thanks!" +) + +// FixSceneOrg returns a working URL if the provided rawURL is a known, +// broken link to a scene.org file. Otherwise it returns the original URL. +// +// For example, the following rawURL: +// +// `http://files.scene.org/view/demos/groups/trsi/ms-dos/trsiscxt.zip` +// +// will return: +// +// `https://files.scene.org/get/demos/groups/trsi/ms-dos/trsiscxt.zip` +func FixSceneOrg(rawURL string) string { + u, err := url.Parse(rawURL) + if err != nil { + return rawURL + } + if u.Host == "scene.org" && u.Path == "/file.php" { + return rawURL + } + if u.Host == "files.scene.org" { + p := u.Path + x := strings.Split(p, "/") + if len(x) > 0 && x[1] == "view" { + x[1] = "get" + newURL := &url.URL{ + Scheme: "https", + Host: "files.scene.org", + Path: strings.Join(x, "/"), + } + return newURL.String() + } + } + return rawURL +} + +// DownloadResponse contains the details of a downloaded file. +type DownloadResponse struct { + ContentLength string // ContentLength is the size of the file in bytes. + ContentType string // ContentType is the MIME type of the file. + LastModified string // LastModified is the last modified date of the file. + Path string // Path is the path to the downloaded file. +} + +// GetFile downloads a file from a remote URL and saves it to the default temp directory. +// If timeout is 0, it uses the default timeout of 5 seconds, otherwise it uses the provided timeout. +// It returns the path to the downloaded file and it should be removed after use. +func GetFile(rawURL string, timeout time.Duration) (DownloadResponse, error) { + url := FixSceneOrg(rawURL) + // Get the remote file + if timeout == 0 { + timeout = Timeout + } + client := http.Client{ + Timeout: timeout, + } + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return DownloadResponse{}, fmt.Errorf("get file new request: %w", err) + } + req.Header.Set("User-Agent", UserAgent) + res, err := client.Do(req) + if err != nil { + return DownloadResponse{}, fmt.Errorf("get file client do: %w", err) + } + defer res.Body.Close() + + download := DownloadResponse{ + ContentLength: res.Header.Get("Content-Length"), + ContentType: res.Header.Get("Content-Type"), + LastModified: res.Header.Get("Last-Modified"), + } + // Create the file in the default temp directory + dst, err := os.CreateTemp(helper.TmpDir(), "get-remotefile-*") + if err != nil { + return DownloadResponse{}, fmt.Errorf("get file create temp: %w", err) + } + defer dst.Close() + + // Write the body to file + if _, err := io.Copy(dst, res.Body); err != nil { + defer os.Remove(dst.Name()) + return DownloadResponse{}, fmt.Errorf("get file io copy: %w", err) + } + download.Path = dst.Name() + return download, nil +} diff --git a/handler/htmx/transfer.go b/handler/htmx/transfer.go index 72617d13..06d953d6 100644 --- a/handler/htmx/transfer.go +++ b/handler/htmx/transfer.go @@ -466,7 +466,7 @@ func (prod Submission) Submit( //nolint:cyclop,funlen return c.String(http.StatusServiceUnavailable, "error, the database commit failed") } - html := fmt.Sprintf("Thanks for the submission of %s production, %d", name, id) + html := fmt.Sprintf("