Reading a salary: the period is looked for across the whole line before it, so it is read wrong and the search is quadratic #420

New issue

Closed

opened 2026-10-01 10:46:34 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:34 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading patterns.salary, running it on sample adverts and timing it on generated pages.

Checked again on 2026-10-01 against main at 0a5cad587: still present. plugins/builtin/ has not changed; with_salary_order is now at jobs/models.py:769-788.

When no period word follows a salary figure, _period falls back to the period its label names ("Annual salary"). The label it is given is not the label: it runs from the start of the line to the end of the salary word. Two things follow. Any "monthly", "daily", "hourly" or "annual" earlier in the same paragraph becomes the salary's period, although the module promises "a period only where the page states one" (plugins/builtin/patterns.py:22-23). And the search over that stretch is quadratic on a long run of whitespace, so one page can hold a worker for minutes. Bounding what _period is given fixes both.

Where:
- plugins/builtin/patterns.py:272-277, _after: line = text.rfind("\n", 0, hit.start()) + 1, then yield text[line : hit.end()], ..., so the label is the whole line up to the salary word.
- plugins/builtin/patterns.py:590, salary: passes f"{label} {filler}" to _period.
- plugins/builtin/patterns.py:540, _period: searches that label with every period pattern when nothing follows the figure. What follows the figure is cut to PERIOD_REACH (:532); the label has no bound.
- plugins/builtin/patterns.py:232-238, _word: a spelling such as "/year" becomes \s*/\s*year, a pattern that starts with an unbounded \s*.
- plugins/builtin/patterns.py:557-560, pay_in: passes everything before the figure (before = text[: match.start()]) to _period in the same way.
- plugins/builtin/htmlutil.py:239, text_of: re.sub(r"[ \t\r\f\v]+", " ", text) collapses ASCII whitespace only, so a run of U+00A0 or any other Unicode space reaches the search intact.

1. A period word anywhere earlier on the line is taken as the salary's period

What happens: patterns.salary(text, "en") reads
- "You get a monthly travel allowance and a salary of €40,000." as €40,000 per month;
- "Hourly paid overtime on top of a salary of €40,000." as per hour;
- "Daily standups. Salary: €40,000" as per day (also through registry.parse_page on a page holding that paragraph: page-metadata, 40000, day);
- "Monthly team lunches | Salary €3,500" as per month.
A line break before the label stops it ("Weekly one-to-ones.\nSalary: €40,000" reads no period), which is why only paragraphs are affected.
Why it matters: the review form arrives with the wrong period selected. Saved unnoticed, the listing's salary is sorted as if multiplied by 220 (day), 1,680 (hour) or 12 (month) (jobs/models.py:766-785, with_salary_order), so a €40,000 salary sorts as €8.8 million a year, and the salary column and filters are wrong for it.

2. The same search is quadratic on a run of non-ASCII spaces

What happens: searching \s*/\s*year and its siblings across a long run of whitespace backtracks over the whole run at every position. patterns.salary("\xa0" * n + "Salary: €40,000 bonus", "en") took 0.11 s, 0.3 s and 1.2 s for n = 2,000, 4,000 and 8,000; patterns.pay_in("\xa0" * n + "€40,000", "en") costs the same. Through registry.parse_page, a page whose one paragraph reads Benefits, then n  , then Salary: €40,000 took 0.35 s, 1.3 s and 5.5 s for n = 4,000, 8,000 and 16,000 (24, 48 and 96 KB); 8,000   (em spaces) took 2 s. Each doubling quadruples the time, so a page of a few hundred kilobytes passes gunicorn's 120 s --timeout (docker/Dockerfile:354).
Who can trigger it: the site behind an address a person captures, or an account holder sending html through POST /api/v1/captures. Only a page with no structured data reaches the fallback that reads the salary this way.
Why it matters: one capture holds a request worker until gunicorn kills it, or an errand worker, which has no limit. No legitimate page needs the search to look further back than the label's own words.

Fix

Give _period only what labels the figure: the matched salary words, the one word immediately before them when nothing but spaces separates it (so "Monthly salary: €3,500", "Gross annual salary: €40,000" and "Hourly wage: £12" keep their period, as they do today), and the filler between the label and the figure (where a period written after the label word, "Salary (monthly): €3,500", sits). Stop at sentence punctuation or a separator such as |. Never pass the line.
Bound pay_in's before the same way.
In _word, write the slash spellings without an unbounded leading class, for example \s{0,2}/\s{0,2}year.
Optionally have text_of collapse every horizontal whitespace character ([^\S\n]+), so that the other readers see one space too.

Done when

A parametrised test in tests/test_page_metadata.py expects an empty period for the four sentences of section 1, and still expects month for "Monthly salary: €3,500", year for "Gross annual salary: €40,000", month for "Salary (monthly): €3,500" and month for "Salary: €40,000 per month".
A test in tests/test_page_metadata.py runs patterns.salary("Benefits" + "\xa0" * 50_000 + "Salary: €40,000", "en") and gets (40000, None, "EUR", "") within a fraction of a second, and the same bound holds for patterns.pay_in with the run before the figure.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `patterns.salary`, running it on sample adverts and timing it on generated pages. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. `plugins/builtin/` has not changed; `with_salary_order` is now at `jobs/models.py:769-788`. When no period word follows a salary figure, `_period` falls back to the period its label names ("Annual salary"). The label it is given is not the label: it runs from the start of the line to the end of the salary word. Two things follow. Any "monthly", "daily", "hourly" or "annual" earlier in the same paragraph becomes the salary's period, although the module promises "a period only where the page states one" (`plugins/builtin/patterns.py:22-23`). And the search over that stretch is quadratic on a long run of whitespace, so one page can hold a worker for minutes. Bounding what `_period` is given fixes both. - **Where:** - `plugins/builtin/patterns.py:272-277`, `_after`: `line = text.rfind("\n", 0, hit.start()) + 1`, then `yield text[line : hit.end()], ...`, so the label is the whole line up to the salary word. - `plugins/builtin/patterns.py:590`, `salary`: passes `f"{label} {filler}"` to `_period`. - `plugins/builtin/patterns.py:540`, `_period`: searches that label with every period pattern when nothing follows the figure. What follows the figure is cut to `PERIOD_REACH` (`:532`); the label has no bound. - `plugins/builtin/patterns.py:232-238`, `_word`: a spelling such as "/year" becomes `\s*/\s*year`, a pattern that starts with an unbounded `\s*`. - `plugins/builtin/patterns.py:557-560`, `pay_in`: passes everything before the figure (`before = text[: match.start()]`) to `_period` in the same way. - `plugins/builtin/htmlutil.py:239`, `text_of`: `re.sub(r"[ \t\r\f\v]+", " ", text)` collapses ASCII whitespace only, so a run of U+00A0 or any other Unicode space reaches the search intact. ## 1. A period word anywhere earlier on the line is taken as the salary's period - **What happens:** `patterns.salary(text, "en")` reads - "You get a monthly travel allowance and a salary of €40,000." as €40,000 per month; - "Hourly paid overtime on top of a salary of €40,000." as per hour; - "Daily standups. Salary: €40,000" as per day (also through `registry.parse_page` on a page holding that paragraph: `page-metadata`, 40000, `day`); - "Monthly team lunches | Salary €3,500" as per month. A line break before the label stops it ("Weekly one-to-ones.\nSalary: €40,000" reads no period), which is why only paragraphs are affected. - **Why it matters:** the review form arrives with the wrong period selected. Saved unnoticed, the listing's salary is sorted as if multiplied by 220 (day), 1,680 (hour) or 12 (month) (`jobs/models.py:766-785`, `with_salary_order`), so a €40,000 salary sorts as €8.8 million a year, and the salary column and filters are wrong for it. ## 2. The same search is quadratic on a run of non-ASCII spaces - **What happens:** searching `\s*/\s*year` and its siblings across a long run of whitespace backtracks over the whole run at every position. `patterns.salary("\xa0" * n + "Salary: €40,000 bonus", "en")` took 0.11 s, 0.3 s and 1.2 s for n = 2,000, 4,000 and 8,000; `patterns.pay_in("\xa0" * n + "€40,000", "en")` costs the same. Through `registry.parse_page`, a page whose one paragraph reads `Benefits`, then n ` `, then `Salary: €40,000` took 0.35 s, 1.3 s and 5.5 s for n = 4,000, 8,000 and 16,000 (24, 48 and 96 KB); 8,000 ` ` (em spaces) took 2 s. Each doubling quadruples the time, so a page of a few hundred kilobytes passes gunicorn's 120 s `--timeout` (`docker/Dockerfile:354`). - **Who can trigger it:** the site behind an address a person captures, or an account holder sending `html` through `POST /api/v1/captures`. Only a page with no structured data reaches the fallback that reads the salary this way. - **Why it matters:** one capture holds a request worker until gunicorn kills it, or an errand worker, which has no limit. No legitimate page needs the search to look further back than the label's own words. ## Fix - Give `_period` only what labels the figure: the matched salary words, the one word immediately before them when nothing but spaces separates it (so "Monthly salary: €3,500", "Gross annual salary: €40,000" and "Hourly wage: £12" keep their period, as they do today), and the filler between the label and the figure (where a period written after the label word, "Salary (monthly): €3,500", sits). Stop at sentence punctuation or a separator such as `|`. Never pass the line. - Bound `pay_in`'s `before` the same way. - In `_word`, write the slash spellings without an unbounded leading class, for example `\s{0,2}/\s{0,2}year`. - Optionally have `text_of` collapse every horizontal whitespace character (`[^\S\n]+`), so that the other readers see one space too. ## Done when - A parametrised test in `tests/test_page_metadata.py` expects an empty period for the four sentences of section 1, and still expects `month` for "Monthly salary: €3,500", `year` for "Gross annual salary: €40,000", `month` for "Salary (monthly): €3,500" and `month` for "Salary: €40,000 per month". - A test in `tests/test_page_metadata.py` runs `patterns.salary("Benefits" + "\xa0" * 50_000 + "Salary: €40,000", "en")` and gets `(40000, None, "EUR", "")` within a fraction of a second, and the same bound holds for `patterns.pay_in` with the run before the figure.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:34 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:46:34 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:56 +00:00

Give the salary period search only the words that label the figure

tiagoagueda closed this issue

2026-10-04 10:04:56 +00:00