Repository navigation
Cover the host-page guard and the profile, and run the e2e suite in parallel - #2225
Conversation
e4ac8fc to
f487f14
Compare
size-limit report 📦
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2225 +/- ##
==========================================
+ Coverage 63.91% 64.16% +0.25%
==========================================
Files 119 119
Lines 2874 2883 +9
Branches 830 791 -39
==========================================
+ Hits 1837 1850 +13
+ Misses 1031 1027 -4
Partials 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c7bc35f to
afca4bf
Compare
94caa51 to
3a5efd9
Compare
Two gaps the browser suite did not reach. A page embedding the widget trusts nothing about who sends it a message, so a foreign frame must not be able to drive it, and the widget must ignore anything that did not come from its own parent. And the profile overlay lists a reader's own comments and paginates them, which nothing exercised. The suite then runs four cases at a time. The constraint was never isolation: every case takes its own browser context, its own thread url and its own anonymous user. It is the auth rate limiter, two requests a second keyed on the client address, which the whole suite shared. Each browser context now carries a distinct forwarded address, so each reader gets its own bucket. That works because the stack configures no trusted proxy, which is what makes the backend believe a forwarding header from any client, and the compose file says so: configuring one collapses every context onto the docker gateway address and puts the whole run in one rate-limit and vote-dedup bucket. The addresses are globally routable in form, since RealIP rejects special-use ranges, and they never leave a loopback-only stack. Only traffic to a remark42 instance is intercepted, because routing everything disables the browser cache and puts unrelated host-page traffic through the driver pipe, which distorts the iframe timing cases. The route pattern and the check inside the handler are built from one list of addresses, so a request the route captures is never one the handler declines. Two harness cases guard the arrangement: one proves the backend gives a forwarded address its own limiter bucket, the other that two browser contexts are served independently. Without them a renamed service or a changed classifier would put the whole run back in one bucket, and the symptom would be rate-limit failures in unrelated cases. Only the last-comments case stays serial, since it reads a site-wide feed.
3a5efd9 to
d7e3759
Compare
umputun
left a comment
There was a problem hiding this comment.
lgtm. The route guards and the parallel isolation hold up. Pinning and read-only are thread-scoped, verification and blocking target anonymous users unique to each case, and the dev user's comments do not reach a count-sensitive assertion: profile pagination signs in as its own anonymous user, and the site-wide feed case stays serial.
one comment is worth correcting. pauseForAuthLimit says the auth bucket is per instance and client IP. Tollbooth also keys it by request path and sets burst to the rate, so with a per-context address 600ms clears the 500ms refill for repeat requests on one path, and different paths never compete. The earlier 700ms failure is not evidence against this arrangement, since every context shared the /auth/status bucket then. The same comment says a fresh context needs no delay while all twelve call sites sleep unconditionally.
Two coverage gaps in the browser suite, and then the suite running four cases at a time.
Coverage
A page embedding the widget trusts nothing about who sends it a message, and neither side of that was exercised: a foreign frame must not be able to drive the host page, and the widget must ignore a message that did not come from its own parent. The profile overlay, which lists a reader's own comments and paginates them, had no browser coverage either.
Parallelism
The suite now runs with
-parallel 4. On one machine a full run goes from 375s to 89s; the CI runner takes about 120s.The constraint was never isolation: each case already took its own browser context, its own thread URL keyed on the test name, and its own anonymous user. It is the auth rate limiter, two requests a second keyed on the client address, which the whole suite shared. Every page load spends some of that budget, so the suite paced itself and the pacing was most of the wall clock.
Each browser context now carries a distinct forwarded client address, so each reader gets its own bucket. That works because the stack configures no trusted proxy, which is what makes the backend honour a forwarding header from any client, and
compose-e2e-test.ymlnow says so in as many words: configuring one collapses every context back onto the docker gateway address and puts the whole run in a single rate-limit and vote-dedup bucket.The addresses are globally routable in form, because
RealIPdeliberately rejects special-use ranges, and they never leave a stack bound to the loopback interface. Only traffic to a remark42 instance is intercepted; routing every request would disable the browser cache and put unrelated host-page traffic through the driver pipe, which distorts the iframe timing cases this suite measures. The route pattern the browser matches on and the check inside the handler are both built from one list of addresses, so a request the route captures can never be one the handler declines.Two harness cases guard the arrangement. One proves the backend gives a forwarded address its own limiter bucket; the other proves two browser contexts are served independently. Without them, a renamed service or a changed address classifier would put the whole run back into one bucket, and the symptom would be rate-limit failures in cases that have nothing to do with the cause.
Only the last-comments case stays serial, since it reads a site-wide feed.
Telegram authentication and subscription coverage is #2228, which builds on this and is blocked until go-pkgz/notify#45 and a go-pkgz/auth release land.