diff --git a/modules/brother/cmd/brother/admin.go b/modules/brother/cmd/brother/admin.go
index ce7aed23..79114abe 100644
--- a/modules/brother/cmd/brother/admin.go
+++ b/modules/brother/cmd/brother/admin.go
@@ -19,8 +19,10 @@ type Admin struct {
HTTP *http.Client
}
-// loginPath is a page that needs a login: asking for it is answered by the login page.
-const loginPath = "/general/status.html"
+// loginStart is where a login starts: the root, which the printer answers with a redirect to the page it serves
+// its login form on (/home/status.html). Asking for a page behind the login instead is answered by a redirect
+// to an error page, and a loginurl naming a page this firmware lacks is a 404 after the login (issue 1514).
+const loginStart = "/"
// maxPage bounds a page of the admin site.
const maxPage = 4 << 20
@@ -76,6 +78,9 @@ func (a *Admin) fetch(ctx context.Context, method, rawURL string, form url.Value
return nil, nil, fmt.Errorf("the page %s is larger than %d bytes", rawURL, maxPage)
}
if resp.StatusCode >= 400 {
+ if resp.StatusCode == http.StatusNotFound {
+ return nil, nil, ¬FoundError{Path: resp.Request.URL.Path}
+ }
return nil, nil, fmt.Errorf("the printer answered %s for %s", resp.Status, resp.Request.URL.Path)
}
return buf, resp.Request.URL, nil
@@ -93,24 +98,33 @@ func passwordForm(p Page) (*Form, *Field) {
return nil, nil
}
-// Login asks for a page that needs one, reads the login form it is answered with, and posts the form's own
-// fields with the password in the field the form names. A login is failed when the answer is a login page
-// again.
+func hasField(f *Form, name string) bool {
+ for _, fl := range f.Fields {
+ if fl.Name == name {
+ return true
+ }
+ }
+ return false
+}
+
+// Login asks for the printer's login page, reads its login form, and posts the form's own fields with the
+// password in the field the form names and the form's loginurl set to the default page, which every firmware has; the page asked for is a request of its own afterwards. A login
+// is failed when the answer is a login page again.
func (a *Admin) Login(ctx context.Context) error {
- body, at, err := a.fetch(ctx, http.MethodGet, a.url(loginPath), nil)
+ body, at, err := a.fetch(ctx, http.MethodGet, a.url(loginStart), nil)
if err != nil {
return fmt.Errorf("the printer's admin page cannot be asked for its login: %w", err)
}
form, pw := passwordForm(ParsePage(body))
if form == nil {
- return fmt.Errorf("the printer's answer to %s holds no login form with a password field", loginPath)
+ return fmt.Errorf("the printer's answer to %s holds no login form with a password field", at.Path)
}
target, err := at.Parse(form.Action)
if err != nil {
return fmt.Errorf("the login form's action %q is not a URL: %w", form.Action, err)
}
- if target.Host != a.Host {
- return fmt.Errorf("the login form posts to %s, not to the printer %s: the password is not sent", target.Host, a.Host)
+ if target.Scheme != "https" || target.Host != a.Host {
+ return fmt.Errorf("the login form posts to %s, not to https on the printer %s: the password is not sent", target.Scheme+"://"+target.Host, a.Host)
}
vals := url.Values{}
for _, f := range form.Fields {
@@ -118,12 +132,21 @@ func (a *Admin) Login(ctx context.Context) error {
vals.Set(f.Name, f.Value)
}
}
+ if _, ok := vals["loginurl"]; ok {
+ vals.Set("loginurl", DefaultPage)
+ }
vals.Set(pw.Name, a.Password)
answer, _, err := a.fetch(ctx, http.MethodPost, target.String(), vals)
if err != nil {
+ // The post may have opened a session before its answer failed: end it if there is one.
+ if home, hat, herr := a.Get(context.Background(), DefaultPage); herr == nil {
+ a.Logout(context.Background(), home, hat)
+ }
return fmt.Errorf("the login was not answered: %w", err)
}
- if again, _ := passwordForm(ParsePage(answer)); again != nil {
+ // A login page again is a form with a password field and the loginurl field; a page that was asked for may
+ // hold a password field of its own.
+ if again, _ := passwordForm(ParsePage(answer)); again != nil && hasField(again, "loginurl") {
return fmt.Errorf("the login failed: the printer showed its login page again, so the secret admin-password is not its password")
}
return nil
@@ -182,3 +205,8 @@ func (a *Admin) Logout(ctx context.Context, page []byte, at *url.URL) string {
}
return "no logout link or form on the page: the session is left to expire on the printer"
}
+
+// notFoundError is the printer's 404 for a page.
+type notFoundError struct{ Path string }
+
+func (e *notFoundError) Error() string { return "the printer answered 404 Not Found for " + e.Path }
diff --git a/modules/brother/cmd/brother/admin_test.go b/modules/brother/cmd/brother/admin_test.go
index f51f3465..bd786582 100644
--- a/modules/brother/cmd/brother/admin_test.go
+++ b/modules/brother/cmd/brother/admin_test.go
@@ -44,8 +44,13 @@ func newFakePrinter(t *testing.T, field, password string) *fakePrinter {
t.Helper()
f := &fakePrinter{field: field, password: password, loggedIn: map[string]bool{}, pages: map[string]string{}}
login := strings.ReplaceAll(adminFixture(t, "login-page.html"), "B1891", field)
+ // The login page the printer serves at /home/status.html carries that page as its loginurl (captured live
+ // 2026-10-20, testdata/admin/live/home_status_html.body); the page it serves at /etc/loginerror.html carries
+ // the page that was asked for.
+ homeLogin := strings.ReplaceAll(login, `value="/general/status.html"`, `value="/home/status.html"`)
synthetic := adminFixture(t, "synthetic-status.html")
- f.pages["/general/status.html"] = synthetic
+ // This firmware has no /general/status.html behind the login (issue 1514): asking for it is a 404.
+ f.pages["/home/status.html"] = synthetic
f.pages["/general/other.html"] = adminFixture(t, "synthetic-settings.html")
authed := func(r *http.Request) bool {
c, err := r.Cookie("session")
@@ -64,7 +69,7 @@ func newFakePrinter(t *testing.T, field, password string) *fakePrinter {
w.Write([]byte(synthetic))
return
}
- w.Write([]byte(login))
+ w.Write([]byte(homeLogin))
return
}
r.ParseForm()
@@ -76,7 +81,8 @@ func newFakePrinter(t *testing.T, field, password string) *fakePrinter {
f.session = "s" + field
f.loggedIn[f.session] = true
http.SetCookie(w, &http.Cookie{Name: "session", Value: f.session, Path: "/"})
- w.Write([]byte(synthetic))
+ // Like Brother's form, a good login goes on to the page named in loginurl.
+ http.Redirect(w, r, r.PostForm.Get("loginurl"), http.StatusFound)
})
mux.HandleFunc("/home/logout.html", func(w http.ResponseWriter, r *http.Request) {
f.mu.Lock()
@@ -89,16 +95,21 @@ func newFakePrinter(t *testing.T, field, password string) *fakePrinter {
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
f.mu.Lock()
defer f.mu.Unlock()
+ if r.URL.Path == "/" {
+ http.Redirect(w, r, "/home/status.html", http.StatusMovedPermanently)
+ return
+ }
f.requests = append(f.requests, r.Method+" "+r.URL.Path)
+ // Captured: a page behind the login is answered by a redirect to the login page whether it exists or not.
+ if !authed(r) {
+ http.Redirect(w, r, "/etc/loginerror.html?url="+url.QueryEscape(r.URL.Path), http.StatusFound)
+ return
+ }
body, ok := f.pages[r.URL.Path]
if !ok {
http.NotFound(w, r)
return
}
- if !authed(r) {
- http.Redirect(w, r, "/etc/loginerror.html?url="+url.QueryEscape(r.URL.Path), http.StatusFound)
- return
- }
w.Write([]byte(body))
})
f.Server = httptest.NewUnstartedServer(mux)
@@ -179,7 +190,7 @@ func TestLoginUsesThePasswordFieldNameTheFormGives(t *testing.T) {
if got := f.posted[0].Get(name); got != "s3cret pass" {
t.Errorf("field %s: the password field holds %q", name, got)
}
- if f.posted[0].Get("loginurl") != "/general/status.html" {
+ if f.posted[0].Get("loginurl") != DefaultPage {
t.Errorf("field %s: the form's hidden loginurl was %q", name, f.posted[0].Get("loginurl"))
}
if len(f.posted[0]) != 2 {
@@ -213,7 +224,7 @@ func TestLogoutEndsTheSessionAndAPageWithoutOneSaysSo(t *testing.T) {
if err := a.Login(ctx); err != nil {
t.Fatal(err)
}
- body, at, err := a.Get(ctx, "/general/status.html")
+ body, at, err := a.Get(ctx, DefaultPage)
if err != nil {
t.Fatal(err)
}
@@ -244,3 +255,48 @@ func TestThePasswordIsNotPostedToAnotherHost(t *testing.T) {
t.Fatalf("error = %v", err)
}
}
+
+// 1514 review: a page the printer does not have leaves no session open, and the error says the page is missing,
+// not that the login failed.
+func TestAMissingPageLeavesNoSessionOpenAndSaysSo(t *testing.T) {
+ f := newFakePrinter(t, "B1891", "pw")
+ _, run := printerTools(t, f, "pw")
+ _, err := run(map[string]any{"page": "/general/nope.html"})
+ if err == nil || !strings.Contains(err.Error(), "the printer has no page /general/nope.html") {
+ t.Fatalf("error = %v, want the printer has no page", err)
+ }
+ if strings.Contains(err.Error(), "login was not answered") {
+ t.Errorf("a missing page is said to be a failed login: %v", err)
+ }
+ if f.loggedInNow() != 0 {
+ t.Errorf("%d sessions are still open on the printer", f.loggedInNow())
+ }
+}
+
+// 1514 review: the password is posted to https on the printer's host only. A form whose action is plain http
+// on the same host, or a root that redirects to plain http, is refused and nothing is posted.
+func TestThePasswordIsNotPostedOverPlainHTTP(t *testing.T) {
+ var posted int
+ mux := http.NewServeMux()
+ page := func(action string) string {
+ return ``
+ }
+ var host string
+ mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodPost {
+ posted++
+ }
+ w.Write([]byte(page("http://" + host + "/home/status.html")))
+ })
+ srv := httptest.NewTLSServer(mux)
+ defer srv.Close()
+ host = strings.TrimPrefix(srv.URL, "https://")
+ a, _ := NewAdmin(host, Fingerprint(srv.TLS.Certificates[0].Certificate[0]), "pw")
+ err := a.Login(context.Background())
+ if err == nil || !strings.Contains(err.Error(), "password is not sent") {
+ t.Errorf("error = %v, want the plain http action refused", err)
+ }
+ if posted != 0 {
+ t.Errorf("%d posts reached the server", posted)
+ }
+}
diff --git a/modules/brother/cmd/brother/deviceproc.go b/modules/brother/cmd/brother/deviceproc.go
index bbd1702d..c615ca30 100644
--- a/modules/brother/cmd/brother/deviceproc.go
+++ b/modules/brother/cmd/brother/deviceproc.go
@@ -334,6 +334,10 @@ func (w *deviceWorker) read(ctx context.Context, page string) (map[string]any, e
if home, hat, herr := a.Get(lctx, DefaultPage); herr == nil {
a.Logout(lctx, home, hat)
}
+ var nf *notFoundError
+ if errors.As(err, &nf) {
+ return nil, fmt.Errorf("the printer has no page %s", page)
+ }
return nil, fmt.Errorf("page %s cannot be read: %w", page, err)
}
logout := a.Logout(lctx, body, at)
diff --git a/modules/brother/cmd/brother/main.go b/modules/brother/cmd/brother/main.go
index 41a75d78..f795bc47 100644
--- a/modules/brother/cmd/brother/main.go
+++ b/modules/brother/cmd/brother/main.go
@@ -165,7 +165,7 @@ func tools(settingsFile string, now func() time.Time) []stdio.Tool {
Input: map[string]any{
"type": "object",
"properties": map[string]any{
- "page": map[string]any{"type": "string", "description": "a page of the admin site: a path starting with / and ending in .html (default /general/status.html)"},
+ "page": map[string]any{"type": "string", "description": "a page of the admin site: a path starting with / and ending in .html (default /home/status.html)"},
},
},
Run: func(args map[string]any) (any, error) {
diff --git a/modules/brother/cmd/brother/printer.go b/modules/brother/cmd/brother/printer.go
index a755b937..546f67f2 100644
--- a/modules/brother/cmd/brother/printer.go
+++ b/modules/brother/cmd/brother/printer.go
@@ -20,7 +20,7 @@ import (
var pagePattern = regexp.MustCompile(`^/[A-Za-z0-9_./-]*\.html$`)
// DefaultPage is the page printer.settings reads when none is asked for.
-const DefaultPage = "/general/status.html"
+const DefaultPage = "/home/status.html"
// The asks the verbs put to the device process (ADR 0468 clause 1; the shape of ADR 0436's mail-sends): the tools
// never hold a credential, so each verb queues an ask on the work-queue stream device-asks, and the device process,
diff --git a/modules/brother/cmd/brother/printer_test.go b/modules/brother/cmd/brother/printer_test.go
index fc73230d..2ef711a3 100644
--- a/modules/brother/cmd/brother/printer_test.go
+++ b/modules/brother/cmd/brother/printer_test.go
@@ -211,3 +211,31 @@ func TestTheAdminWorkerSettlesEachAsk(t *testing.T) {
t.Errorf("an answer that cannot be written: action %v delay %v, want retry", act, delay)
}
}
+
+// 1514: printer.settings with no page, and with /home/status.html, logs in from the page the printer serves its
+// login form on (the root redirects to it) and reads that page. The printer has no /general/status.html behind
+// the login: the fake, like the captured printer, answers it a 404 once logged in, and a redirect to the login
+// page before.
+func TestPrinterSettingsDefaultPageAndHomeStatusAreReadAfterTheLogin(t *testing.T) {
+ for name, args := range map[string]map[string]any{"default": {}, "explicit": {"page": "/home/status.html"}} {
+ f := newFakePrinter(t, "B1891", "pw")
+ _, run := printerTools(t, f, "pw")
+ got, err := run(args)
+ if err != nil {
+ t.Fatalf("%s: %v", name, err)
+ }
+ raw, _ := json.Marshal(got)
+ if !strings.Contains(string(raw), "Synthetic status") || !strings.Contains(string(raw), "/home/status.html") {
+ t.Errorf("%s: answer %.200s", name, raw)
+ }
+ if len(f.posted) != 1 || f.posted[0].Get("loginurl") != "/home/status.html" {
+ t.Errorf("%s: posted %v, want loginurl /home/status.html", name, f.posted)
+ }
+ if f.loggedInNow() != 0 {
+ t.Errorf("%s: the session is still open", name)
+ }
+ }
+ if DefaultPage != "/home/status.html" {
+ t.Errorf("DefaultPage = %s", DefaultPage)
+ }
+}
diff --git a/modules/brother/cmd/brother/testdata/admin/live/etc_loginerror_html_url__2Fgeneral_2Fstatus_2Ehtml.body b/modules/brother/cmd/brother/testdata/admin/live/etc_loginerror_html_url__2Fgeneral_2Fstatus_2Ehtml.body
new file mode 100644
index 00000000..9ac63bd5
--- /dev/null
+++ b/modules/brother/cmd/brother/testdata/admin/live/etc_loginerror_html_url__2Fgeneral_2Fstatus_2Ehtml.body
@@ -0,0 +1,3 @@
+
+
+Brother MFC-L8390CDW series
