rouggy/OpsLog
0
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 74 Wiki Activity

fix(clublog): say what is wrong, not four kilobytes of markup

Browse Source 
Club Log refuses with its ordinary web page rather than an error string, so the
new test reported a rejected login by pasting a 403 page — title, stylesheets,
navigation and all — into the status bar. The body now goes to the log, where a
real diagnosis happens, and the operator gets the one sentence there is to act
on: check the e-mail, the password and the logbook callsign.

Dropped the startyear=2099 filter with it, and that one matters more than it
looks. It was there to keep the reply small, but it was never verified against
Club Log's API — and Club Log answers an unrecognised request with the SAME 403
it uses for a refused login. An unverified parameter would therefore have made
every CORRECT password look wrong, which is precisely the failure this change
set out to end. The reply is capped at 4 KB and closed at once instead; the
status code arrives ahead of the body either way.
This commit is contained in:
rouggy
2026-08-11 15:34:06 +02:00
parent e107b0b741
commit 1ac00c101e
2 changed files with 51 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/extsvc/clublog.go
+25 -8
View File
@@ -27,6 +27,14 @@ const clublogBatchURL = "https://clublog.org/putlogs.php"
// must send a real, app-identifying User-Agent. // must send a real, app-identifying User-Agent.
const clublogUserAgent = "OpsLog/1.0 (+https://github.com/GregTroar/OpsLog)" const clublogUserAgent = "OpsLog/1.0 (+https://github.com/GregTroar/OpsLog)"
// looksLikeHTML reports a body that is a web page rather than an answer. Club
// Log serves its normal site for refusals and blocks, so this is what separates
// "here is what went wrong" from 4 KB of markup an operator cannot act on.
func looksLikeHTML(s string) bool {
l := strings.ToLower(strings.TrimSpace(s))
return strings.HasPrefix(l, "<!doctype html") || strings.HasPrefix(l, "<html")
}
// clublogDownloadURL is Club Log's ADIF export. Used ONLY to verify credentials // clublogDownloadURL is Club Log's ADIF export. Used ONLY to verify credentials
// (see TestClublog): it is the one authenticated endpoint that cannot change // (see TestClublog): it is the one authenticated endpoint that cannot change
// anything in the operator's log, which is what a test button must never do. // anything in the operator's log, which is what a test button must never do.
@@ -244,10 +252,12 @@ func TestClublog(ctx context.Context, cfg ServiceConfig) (string, error) {
form.Set("email", email) form.Set("email", email)
form.Set("password", cfg.Password) form.Set("password", cfg.Password)
form.Set("call", call) form.Set("call", call)
// A future start date: the credentials are what is being checked, not the // No date filter. A "startyear=2099" was tried to keep the reply small, but
// log, and an operator with 130 000 QSOs should not download them to find // Club Log answers an unrecognised request with the SAME 403 it uses for a
// out whether a password is right. // refused login — so an unverified parameter would have made every correct
form.Set("startyear", "2099") // password look wrong, which is the failure this whole change exists to end.
// The reply is capped at 4 KB and the body closed immediately instead; the
// status code arrives before any of it.
req, err := http.NewRequestWithContext(ctx, http.MethodPost, clublogDownloadURL, strings.NewReader(form.Encode())) req, err := http.NewRequestWithContext(ctx, http.MethodPost, clublogDownloadURL, strings.NewReader(form.Encode()))
if err != nil { if err != nil {
@@ -268,11 +278,18 @@ func TestClublog(ctx context.Context, cfg ServiceConfig) (string, error) {
case http.StatusOK: case http.StatusOK:
return fmt.Sprintf("Ready — %s via %s (Club Log accepted the login)", call, email), nil return fmt.Sprintf("Ready — %s via %s (Club Log accepted the login)", call, email), nil
case http.StatusUnauthorized, http.StatusForbidden: case http.StatusUnauthorized, http.StatusForbidden:
if msg == "" { // Club Log refuses with its ordinary web page, not an error string, so the
msg = "wrong e-mail, password or logbook callsign" // body is 4 KB of markup that says nothing to an operator. It goes to the
} // log — that is where a real diagnosis happens — and the message says the
return "", fmt.Errorf("Club Log rejected the login: %s", msg) // one thing there is to do about it.
LogSink("clublog: login refused (http %d), body: %s", resp.StatusCode, msg)
return "", fmt.Errorf("Club Log refused the login — check the account e-mail, " +
"the password and the logbook callsign. They are the Club Log website's own credentials")
default: default:
LogSink("clublog: unexpected http %d, body: %s", resp.StatusCode, msg)
if looksLikeHTML(msg) {
return "", fmt.Errorf("clublog: Club Log answered with a web page (http %d) instead of a log — see the log file", resp.StatusCode)
}
if len(msg) > 200 { if len(msg) > 200 {
msg = msg[:200] + "…" msg = msg[:200] + "…"
} }
internal/extsvc/clublog_test.go
+26
View File
@@ -0,0 +1,26 @@
package extsvc
import "testing"
// Club Log answers a refused login with its ordinary web page. The operator must
// get a sentence they can act on, not four kilobytes of markup — that was the
// first thing reported once the test started working at all.
func TestLooksLikeHTML(t *testing.T) {
cases := []struct {
body string
want bool
}{
{"<!DOCTYPE html>\n<html lang='en'>…403 - Access denied…", true},
{" <html><head><title>403</title></head></html>", true},
{"<HTML>", true},
{"Invalid credentials", false},
{"", false},
// An ADIF answer must never be mistaken for a page.
{"<eoh>\n<call:5>F4BPO <band:3>20m <eor>", false},
}
for _, c := range cases {
if got := looksLikeHTML(c.body); got != c.want {
t.Errorf("looksLikeHTML(%.40q) = %v, want %v", c.body, got, c.want)
}
}
}