Fix manage handlers ignoring multipart form bodies
The /manage page posts with FormData, i.e. multipart/form-data, but the edit, delete, tags, restore and empty-trash handlers parsed with r.ParseForm, which only reads urlencoded bodies. So every field came back empty: delete returned 200 with deleted:0 and moved nothing, edits saved nothing — while curl -d (urlencoded) worked, which is why tests and manual checks missed it. Parse with ParseMultipartForm (tolerating ErrNotMultipart so urlencoded still works), via a shared parseForm helper used by all manage handlers. handleUpload already did this. Also stop the UI reporting deleted:0 / updated:0 as success, and add a multipart regression test.
This commit is contained in:
1 parent
25384d3f3b
commit
bf75c9fe8e
4 files changed
+75
-9
No files matched your search
@@ -69,7 +69,7 @@ func (s *Server) handleEdit(w http.ResponseWriter, r *http.Request) {
|
|||||||
if s.rejectCrossSite(w, r) {
|
if s.rejectCrossSite(w, r) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := r.ParseForm(); err != nil {
|
if err := parseForm(r); err != nil {
|
||||||
http.Error(w, "bad request", http.StatusBadRequest)
|
http.Error(w, "bad request", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -191,6 +191,19 @@ func (s *Server) handleBulkTags(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
// ── helpers ────────────────────────────────────────────────────────────────
|
// ── helpers ────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
// parseForm reads form values from either an application/x-www-form-urlencoded
|
||||||
|
// or a multipart/form-data body. Browsers posting FormData send multipart,
|
||||||
|
// which r.ParseForm alone does not read from the body; ParseMultipartForm
|
||||||
|
// handles both — it runs ParseForm internally (so query and urlencoded values
|
||||||
|
// are parsed) and only reports ErrNotMultipart for a non-multipart body, by
|
||||||
|
// which point those values are already available.
|
||||||
|
func parseForm(r *http.Request) error {
|
||||||
|
if err := r.ParseMultipartForm(32 << 20); err != nil && err != http.ErrNotMultipart {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// resolveSlug reports whether slug names an existing post and returns the image
|
// resolveSlug reports whether slug names an existing post and returns the image
|
||||||
// path. It rejects anything that is not a bare, safe filename, closing path
|
// path. It rejects anything that is not a bare, safe filename, closing path
|
||||||
// traversal and access to dotfiles such as the .trash directory.
|
// traversal and access to dotfiles such as the .trash directory.
|
||||||
@@ -210,7 +223,7 @@ func (s *Server) resolveSlug(slug string) (string, bool) {
|
|||||||
// formSlugs collects slugs from repeated `slug` fields and/or a comma-separated
|
// formSlugs collects slugs from repeated `slug` fields and/or a comma-separated
|
||||||
// `slugs` field, de-duplicated.
|
// `slugs` field, de-duplicated.
|
||||||
func (s *Server) formSlugs(r *http.Request) ([]string, error) {
|
func (s *Server) formSlugs(r *http.Request) ([]string, error) {
|
||||||
if err := r.ParseForm(); err != nil {
|
if err := parseForm(r); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
seen := make(map[string]bool)
|
seen := make(map[string]bool)
|
||||||
|
|||||||
@@ -1,9 +1,11 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"image"
|
"image"
|
||||||
"image/png"
|
"image/png"
|
||||||
|
"mime/multipart"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -58,6 +60,20 @@ func form(v url.Values) *http.Request {
|
|||||||
return r
|
return r
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// multipartForm builds a multipart/form-data request, as a browser's FormData
|
||||||
|
// does — the content type the manage handlers must parse, not just urlencoded.
|
||||||
|
func multipartForm(fields map[string]string) *http.Request {
|
||||||
|
var b bytes.Buffer
|
||||||
|
mw := multipart.NewWriter(&b)
|
||||||
|
for k, v := range fields {
|
||||||
|
mw.WriteField(k, v)
|
||||||
|
}
|
||||||
|
mw.Close()
|
||||||
|
r := httptest.NewRequest(http.MethodPost, "/", &b)
|
||||||
|
r.Header.Set("Content-Type", mw.FormDataContentType())
|
||||||
|
return r
|
||||||
|
}
|
||||||
|
|
||||||
func readMeta(t *testing.T, content, slug string) PhotoMeta {
|
func readMeta(t *testing.T, content, slug string) PhotoMeta {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
var m PhotoMeta
|
var m PhotoMeta
|
||||||
@@ -213,6 +229,37 @@ func TestHandleDeleteMovesToTrashAndPrunes(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestMultipartBodies guards the regression where the manage handlers used
|
||||||
|
// r.ParseForm (urlencoded only) and silently ignored the multipart bodies that
|
||||||
|
// browsers send via FormData, returning 200 with deleted:0 / no change.
|
||||||
|
func TestMultipartBodies(t *testing.T) {
|
||||||
|
s, content, _ := newTestServer(t)
|
||||||
|
|
||||||
|
// Edit via multipart.
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
s.handleEdit(rec, multipartForm(map[string]string{
|
||||||
|
"slug": "foo", "caption": "Multipart", "date": "2021-02-03", "tags": "x,y",
|
||||||
|
}))
|
||||||
|
if rec.Code != http.StatusOK {
|
||||||
|
t.Fatalf("multipart edit: code=%d", rec.Code)
|
||||||
|
}
|
||||||
|
if m := readMeta(t, content, "foo"); m.Caption != "Multipart" || strings.Join(m.Tags, ",") != "x,y" {
|
||||||
|
t.Errorf("multipart edit not applied: %+v", m)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Delete via multipart actually moves the file.
|
||||||
|
rec = httptest.NewRecorder()
|
||||||
|
s.handleDelete(rec, multipartForm(map[string]string{"slug": "bar"}))
|
||||||
|
var res map[string]int
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &res)
|
||||||
|
if res["deleted"] != 1 {
|
||||||
|
t.Fatalf("multipart delete: deleted=%d, want 1 (body=%s)", res["deleted"], rec.Body)
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(filepath.Join(content, "bar.png")); !os.IsNotExist(err) {
|
||||||
|
t.Error("bar.png should have moved to trash")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCSRFGuard(t *testing.T) {
|
func TestCSRFGuard(t *testing.T) {
|
||||||
s, _, _ := newTestServer(t) // generator.baseURL == "http://x"
|
s, _, _ := newTestServer(t) // generator.baseURL == "http://x"
|
||||||
|
|
||||||
|
|||||||
+11
-5
@@ -212,7 +212,9 @@
|
|||||||
const form = new FormData();
|
const form = new FormData();
|
||||||
form.append('slug', slug);
|
form.append('slug', slug);
|
||||||
try {
|
try {
|
||||||
await post('/api/delete', form);
|
const res = await post('/api/delete', form);
|
||||||
|
const j = await res.json().catch(() => ({}));
|
||||||
|
if (!j.deleted) { flash('Not deleted — "' + slug + '" not found on the server', false); return; }
|
||||||
el.remove();
|
el.remove();
|
||||||
flash('Moved ' + slug + ' to trash', true);
|
flash('Moved ' + slug + ' to trash', true);
|
||||||
refreshBulk();
|
refreshBulk();
|
||||||
@@ -233,8 +235,10 @@
|
|||||||
const form = new FormData();
|
const form = new FormData();
|
||||||
form.append('slugs', slugs.join(','));
|
form.append('slugs', slugs.join(','));
|
||||||
try {
|
try {
|
||||||
await post('/api/delete', form);
|
const res = await post('/api/delete', form);
|
||||||
flash('Moved ' + slugs.length + ' to trash', true);
|
const j = await res.json().catch(() => ({}));
|
||||||
|
if (!j.deleted) { flash('Nothing deleted — slugs not found on the server', false); return; }
|
||||||
|
flash('Moved ' + j.deleted + ' to trash', true);
|
||||||
await load();
|
await load();
|
||||||
} catch (err) { flash('Delete failed: ' + err.message, false); }
|
} catch (err) { flash('Delete failed: ' + err.message, false); }
|
||||||
});
|
});
|
||||||
@@ -247,9 +251,11 @@
|
|||||||
form.append('slugs', slugs.join(','));
|
form.append('slugs', slugs.join(','));
|
||||||
form.append(label, val);
|
form.append(label, val);
|
||||||
try {
|
try {
|
||||||
await post('/api/tags', form);
|
const res = await post('/api/tags', form);
|
||||||
|
const j = await res.json().catch(() => ({}));
|
||||||
|
if (!j.updated) { flash('No posts updated', false); return; }
|
||||||
document.getElementById(field).value = '';
|
document.getElementById(field).value = '';
|
||||||
flash('Updated tags on ' + slugs.length + ' post(s)', true);
|
flash('Updated tags on ' + j.updated + ' post(s)', true);
|
||||||
await load();
|
await load();
|
||||||
} catch (err) { flash('Tag update failed: ' + err.message, false); }
|
} catch (err) { flash('Tag update failed: ' + err.message, false); }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -135,7 +135,7 @@ func (s *Server) handleRestore(w http.ResponseWriter, r *http.Request) {
|
|||||||
if s.rejectCrossSite(w, r) {
|
if s.rejectCrossSite(w, r) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := r.ParseForm(); err != nil {
|
if err := parseForm(r); err != nil {
|
||||||
http.Error(w, "bad request", http.StatusBadRequest)
|
http.Error(w, "bad request", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -175,7 +175,7 @@ func (s *Server) handleEmptyTrash(w http.ResponseWriter, r *http.Request) {
|
|||||||
if s.rejectCrossSite(w, r) {
|
if s.rejectCrossSite(w, r) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := r.ParseForm(); err != nil {
|
if err := parseForm(r); err != nil {
|
||||||
http.Error(w, "bad request", http.StatusBadRequest)
|
http.Error(w, "bad request", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user